Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reusable Travis CI setup #99

Closed
honzajavorek opened this issue Oct 19, 2017 · 21 comments
Closed

Reusable Travis CI setup #99

honzajavorek opened this issue Oct 19, 2017 · 21 comments

Comments

@honzajavorek
Copy link
Contributor

Thanks for commitlint and for the CI setup tutorial! I like the pre-prepared shell script, which counts with all the possibilities which can happen when using GitHub and Travis CI, but currently I can only use it as part of my project by copy-pasting from the docs.

Since it's not two lines, but a bunch of them, many if/elses included, I'm wondering whether it would make sense to turn it into a separate package. I want to install commitlint on multiple repositories and I believe being able to do just sth like npm i commitlint-travis --save-dev instead of copy-pasting the script would make the maintenance of the repos easier as well as doing any updates if e.g. Travis CI or GitHub make any changes and the flow breaks.

What do you think? I'm happy to help.

@marionebl
Copy link
Contributor

marionebl commented Oct 20, 2017

That is a good idea, setting up CI should really be easier. I wonder if a cli wrapper is the appropriate way to do this, though - we'd end up with commitlint-circle-ci, commitlint-appveyor, etc.

In my mind the current CI script concerns itself with determining the appropriate commit ranges for a (Travis) CI environment.

Perhaps a tool that does exactly that and plugs nicely intocommitlint would be the scaleable approach.

commit-range # detects CI environments automatically?
{from: 'a9385', to: 'e2357'}

# this api is just my cranky first shot
commitlint --range $(commit-range)

Let me know what you think!

@felixfbecker
Copy link

I am probably missing something, but I wonder why the script is so complicated? Shouldn't only $TRAVIS_COMMIT_RANGE contain exactly the information that's needed?

@marionebl
Copy link
Contributor

marionebl commented Oct 21, 2017

@felixfbecker If recall correctly there were unsolved issues with $TRAVIS_COMMIT_RANGE: travis-ci/travis-ci#4596

@felixfbecker
Copy link

That can be worked around with by doing ${TRAVIS_COMMIT_RANGE/.../..} as stated in that thread

@honzajavorek
Copy link
Contributor Author

I like the idea of the standalone commit-range "calculator" and --range being the API between those two. Of course, if the tool would be redundant, because sth. like ${TRAVIS_COMMIT_RANGE/.../..} is enough, that would be even better. But I'm no Travis CI commit range guru, so I'd leave that to others to verify.

@honzajavorek
Copy link
Contributor Author

So far I came up with following simplification: apiaryio/dredd-transactions#116

#!/bin/bash
set -e
git remote set-branches origin master
git fetch --unshallow --quiet
git checkout master --quiet
git checkout - --quiet
./node_modules/.bin/commitlint --from=master --to="$TRAVIS_COMMIT"

I'm not sure whether this can be further simplified, but it seems to work for all the scenarios when I played with it.

@felixfbecker
Copy link

@honzajavorek what if the PR base is not master?

@honzajavorek
Copy link
Contributor Author

That's good point. Doesn't happen very often, but happens to me as well 🤔 I guess using sth like ${TRAVIS_PULL_REQUEST_BRANCH:='master'} instead of just master could do the job.

@marionebl
Copy link
Contributor

Hey, I took your improved script @honzajavorek and ran with it here:

Contrary to what I wrote in https://github.com/marionebl/commitlint/issues/99#issuecomment-338120450, I went with a spoecialized @commitlint/travis-cli package that exposes travis-commitlint.

Some code review on that PR would be super nice 🙇

It is intended to be used with zero configuration. I wonder if you would be interested in testing out a prerelease of this @honzajavorek @felixfbecker.

@felixfbecker
Copy link

I would definitely test this! Didn't use commitlint so far for just this reason - I don't want to have a big bash script in every repo.

@marionebl
Copy link
Contributor

Hey there, you can try this via

npm install --save-dev @commitlint/travis-cli
# travis.yml
script:
  - commitlint-travis
  - npm test

@marionebl
Copy link
Contributor

marionebl commented Nov 24, 2017

First issue detected
Fails for tag commits https://travis-ci.org/marionebl/commitlint/jobs/306828679#L642

@felixfbecker
Copy link

I am a bit worried about the versioning - does this mean that the version of commit-cli used is dictated by the version that is defined as a dependency in commitlint-travis? Couldn't we instead make commitlint-travis only concerned with figuring out the range? e.g.

script:
  - commitlint --range $(commitlint-travis)

@ChristianMurphy
Copy link
Contributor

another issue run across:

/home/travis/build/marionebl/commitlint/@commitlint/cli/lib/cli.js:67
		throw err;
		^
Error: fatal: Invalid revision range master..c91cd18860960cb4b51941464e95b289482b4b6b
    at DestroyableTransform._transform (/home/travis/build/marionebl/commitlint/node_modules/@marionebl/git-raw-commits/index.js:69:30)
    at DestroyableTransform.Transform._read (/home/travis/build/marionebl/commitlint/node_modules/readable-stream/lib/_stream_transform.js:182:10)
    at DestroyableTransform.Transform._write (/home/travis/build/marionebl/commitlint/node_modules/readable-stream/lib/_stream_transform.js:170:83)
    at doWrite (/home/travis/build/marionebl/commitlint/node_modules/readable-stream/lib/_stream_writable.js:406:64)
    at writeOrBuffer (/home/travis/build/marionebl/commitlint/node_modules/readable-stream/lib/_stream_writable.js:395:5)
    at DestroyableTransform.Writable.write (/home/travis/build/marionebl/commitlint/node_modules/readable-stream/lib/_stream_writable.js:322:11)
    at Socket.ondata (_stream_readable.js:639:20)
    at emitOne (events.js:121:20)
    at Socket.emit (events.js:211:7)
    at addChunk (_stream_readable.js:263:12)
{ Error: Command failed: /home/travis/build/marionebl/commitlint/@commitlint/cli/lib/cli.js --from master --to c91cd18860960cb4b51941464e95b289482b4b6b
    at Promise.all.then.arr (/home/travis/build/marionebl/commitlint/node_modules/execa/index.js:236:11)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
  code: 1,
  killed: false,
  stdout: null,
  stderr: null,
  failed: true,
  signal: null,
  cmd: '/home/travis/build/marionebl/commitlint/@commitlint/cli/lib/cli.js --from master --to c91cd18860960cb4b51941464e95b289482b4b6b',
  timedOut: false }
The command "node @commitlint/travis-cli/lib/cli.js" exited with 1.

E.G. https://travis-ci.org/marionebl/commitlint/jobs/306896748#L636-L666

@marionebl
Copy link
Contributor

@ChristianMurphy I believe this is related to the commit being unavailable locally, will require special handling of fork PR commits

@marionebl
Copy link
Contributor

@felixfbecker Yes, I intended to couple travis-cli and cli by design to keep maintenance efforts down. Do you have pressing requirements that make the decoupled variant necessary?

@felixfbecker
Copy link

felixfbecker commented Nov 30, 2017

My fear is that a new version of commit-cli is released but travis-cli is still depending on the older version. Or when I get a Greenkeeper update, I don't actually get a changelog in the PR, because the only real update happened in the other dependency. Besides, this tool could have use cases for more than commit-cli - it can just be a standalone CLI that reads travis env vars and outputs a git commit range on stdout. No need to couple them together.

@marionebl
Copy link
Contributor

marionebl commented Nov 30, 2017

@ChristianMurphy @honzajavorek @felixfbecker I released a refactored version of travis-cli via 5.2.2. (This stuff is hard to test properly)

The refactored version now uses TRAVIS_COMMIT_RANGE (thanks @felixfbecker for the tip)

I'll do testing via local usage here, feedback from your repos would be very welcome!

@marionebl
Copy link
Contributor

@felixfbecker Okay, understood. I'll reconsider this when travis-cli is stable enough to be actually usable.

@ChristianMurphy
Copy link
Contributor

My fear is that a new version of commit-cli is released but travis-cli is still depending on the older version.

My understanding is: Lerna takes care of keeping the @commitlint dependencies in sync automatically as part of the release process. So out of sync dependencies should not be possible.

@honzajavorek
Copy link
Contributor Author

@marionebl I'm sorry for not helping yet, there have been some vacations etc. I'm glad this moves forward and I'll try to do my best to try the new tool out ASAP to provide feedback.

marionebl added a commit that referenced this issue Dec 26, 2017
marionebl added a commit that referenced this issue Jan 9, 2018
bpedersen pushed a commit to bpedersen/commitlint that referenced this issue Oct 15, 2019
…t/npm_and_yarn/lodash.template-4.5.0

chore(deps): bump lodash.template from 4.4.0 to 4.5.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants