-
Notifications
You must be signed in to change notification settings - Fork 113
Move to kcd-scripts #86
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
Conversation
Codecov Report
@@ Coverage Diff @@ ## master #86 +/- ## ===================================== Coverage 100% 100% ===================================== Files 1 1 Lines 63 63 Branches 11 10 -1 ===================================== Hits 63 63
Continue to review full report at Codecov.
|
I''ll shamelessly ping @kentcdodds to see if he can provide some insights with regarding testing the Travis part (especially the semantic-release one, which is the main goal of this PR. See #85).
🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks cool. kcd-scripts is pretty nice 😉
As far as testing, it's hard to test Travis. I normally just do trial and error 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this will need to be changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, how come? I was expecting Travis + semantic release will set the version right before it releases.
Actually, I wanted to ask you something. I saw that semantic-release uses git commit messages to compute the required version bump. I've noticed, however, that you are not enforcing any commit structure in RTL, for instance (example). How does that work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry Kent, I thought you meant the version field, now I see you added your comment two lines below. I double checked, and since the file is called src/vue-testing-library.js, the generated dist/ folder keeps the same naming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I merge pill requests I use the "Squash and merge" option which allows me to write my own commit message. Much less of a burden on contributors and I don't need commitizen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, do we have a workflow for commit -> release now?
This is great, do we have a workflow for commit -> release now?
So the idea is that we set VTL repo to only work with "squash and merge" strategy (削除) (this can be done through config, no problem) (削除ここまで) (update: done), so that we get to author the commit message that merges to master.
This way, by following semantic-release guidelines, Travis script will automatically run tests, update to the appropriate package version, and deploy it.
This remove burden from contributors, because we don't have to set commitizen and make sure everyone is properly tagging their commits.
At least this is the idea, can't (obviously) test it until everything gets merged and we see the results 😇
If that sounds reasonable, I'd say we merge this as a patch version - in this case, I'd use a commit message that reads "chore: move building process to kcd-scripts" - and see how it goes. A lot of things (github/npm tokens, config, etc) could go wrong, but we have to try :)
Okay I'm giving this a go. Wish me luck 😂
Okay, once I created the appropriate git tag (v2.0.1) to match the current published version, looks like the script is failing at npm publish (after properly inferring the next version and so on).
[release] npm notice [release] npm ERR! code E404 [release] npm ERR! 404 Not Found - PUT https://registry.npmjs.org/@testing-library%2fvue - Not found [release] npm ERR! 404 [release] npm ERR! 404 '@testing-library/vue@2.1.0' is not in the npm registry.
It is weird because the URL actually exists and the library is obviously published 🤷♀
You have the GH_TOKEN and NPM_TOKEN set right?
NPM_TOKEN was created/uploaded when I ran semantic-release-cli setup, but I just noticed that I'm not "owner" of VTL in npm (just checked by running npm owner ls @testing-library/vue). That should make a difference, right?
I just created and added an NPM_TOKEN using the testing-library-bot account which should have access. We should be good to go now.
That was it @kentcdodds 💯💯💯💯 thank you!!
weyert
commented
Aug 18, 2019
Cool! Great job guys! Does anyone know if you can zennify the squash and merge commit message, though?
I don't know if any way to automate it, but I don't see any reason to either 🤷♂️
weyert
commented
Aug 18, 2019
I was just wondering, we are switching to Github at work and if you squash and merge allows to change the commit message. Guess it would be nice to avoid fix it messages 😀
Yep, it does allow you to change the message when you squash and merge 👍
Uh oh!
There was an error while loading. Please reload this page.
By using
kcd-scriptswe stay one step closer to Testing Library sibling packages while removing the need to handle stuff manually.kcd-scriptsto manage npm scripts.src/__tests__to honor default config.npm run format(this is why there's a lot of small styling changes - mostly trailing commas and whitespaces around brackets).Is this a Breaking Change?:
I'd love to make sure that the
buildoutput is still okay. I removedbrowsersfrom.babelrcto use the default config (the same used in RTL, for instance).