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

Update on npm #16

Open
MrEbbinghaus opened this issue Jul 15, 2019 · 17 comments
Open

Update on npm #16

MrEbbinghaus opened this issue Jul 15, 2019 · 17 comments

Comments

@MrEbbinghaus
Copy link

Hey,

I like your engagement to fix up bootstrap-tour.

Anyway, would you mind updating the project on npm?

npm publish from here: #3 (comment)

PS:
Maybe you want to add a CI job to automate the publish job.
https://circleci.com/blog/publishing-npm-packages-using-circleci-2-0/

@IGreatlyDislikeJavascript
Copy link
Owner

IGreatlyDislikeJavascript commented Jul 15, 2019

Hi, thanks for your comments, glad the updates are useful.

THE SHORT VERSION of this post: is it normal practice to publish npm for all commits, or just releases?

The long version:

I'm happy to update the npm but I need some guidance here please. My understanding is that this will be an "automated" push to anyone who uses npm. So far I've only published an npm for the one formally tested Tourist release, which I know is bug-free.

My concern is that if I publish npms (or use the automated tool you suggest) for all commits, rather than just release versions, I may end up pushing a broken version by accident.

For example, we've just recently closed an issue/pull for button labels, where the original pull caused errors on some systems. I wouldn't want to push that to npm and people have it into their sites when it's not working.

This is why I was cautious about providing npm, webjar etc. The purpose of Tourist is to update the original Bootstrap Tour, fix issues, add support for BS4 and migrate to native ES6 etc. So it is very much an "in progress" plugin, which is why I don't even provide a minified version - to underline that point.

So please can I get some advice and feedback on this. What are the implications of updating the npm with commits and not just releases? Is there a better way to handle this?

(Note: as per readme I'm not a JavaScript coder, I'm an old x86 asm and c++ coder from the 90s who simply needed original Tour fixed!)

@MrEbbinghaus
Copy link
Author

You should release every stable release.


An automatic tool could work on every github release or git tag.

In the example from circleci above:

filters:
            tags:
              only: /^v.*/
            branches:
              ignore: /.*/

This means:

  1. ignore every branch (normaly the build would trigger on every commit)
  2. build only on git tags starting with ˋvˋ

Tags: https://git-scm.com/book/en/v2/Git-Basics-Tagging

@IGreatlyDislikeJavascript
Copy link
Owner

That's very useful, thank you. I'm going to keep this issue open for now as a reminder that we need to set a stable release point and update the npm, probably after thenewbeat's current pull (addStep) is incorporated.

@IGreatlyDislikeJavascript
Copy link
Owner

v0.2.0 published as release on github and on npm.

@tszulc
Copy link
Contributor

tszulc commented Oct 31, 2019

@IGreatlyDislikeJavascript would you be able to update the version on npm again?

https://www.npmjs.com/package/bootstrap-tourist
Screen Shot 2019-10-31 at 13 55 07

@IGreatlyDislikeJavascript
Copy link
Owner

@tszulc I don't want to update to v0.3.0 on npm just yet, because people may pick it up automatically. Although we've tested v0.3.0 a lot, it's a fundamental change to how the visual appearance is created. I'd prefer to wait a bit longer before pushing to npm (and risking breaking someone's site inadvertently). We've already had one bug report due to an interesting z-index issue, currently under investigation.

If you think I'm wrong and I should push it, please do let me know - as I've said many times I'm not really a js or web coder, so don't necessarily know the right thing to do in these cases.

@tszulc
Copy link
Contributor

tszulc commented Nov 4, 2019

@IGreatlyDislikeJavascript typically if a version is released/tagged on github, it's usually available on whichever package manager(s) shortly afterwards. However, since we're not fully following the semantic versioning spec, I agree with the idea of waiting for a fix. Going forward I would highly recommend that we do the following:

  1. follow semantic versioning, since it's supposed to inform people about major, minor, and patch updates, and should reduce the risk of breaking someone's site
  2. include a package.json file in the repository
  3. switch to automated releases to npm, based on releases/tags on github

@IGreatlyDislikeJavascript
Copy link
Owner

Okay, thank you. I've screwed this up again, annoyingly, despite my best effort.

The semantic versioning thing confuses me. I know the principle isn't difficult to understand, but my logic was simple - and apparently wrong. I moved to v0.3.0 because:

  • this wasn't a major breaking change (so keep 0.x.x)
  • it was a measurable change to backdrop functionality (so increment to 0.3.x)
  • there were no patches in this version (so keep x.x.0)

But then I see your point about releases etc.

As for package.json and automated releases, I probably won't be doing that but I'm very happy for someone else to take on responsibility.

I've added a note to the readme: Policy on npm, semantic versioning etc

@tszulc
Copy link
Contributor

tszulc commented Nov 5, 2019

That's alright.

I've made a PR that updates the README, adds a CHANGELOG, and a package.json, as well as adds a script to allow cutting a release: #41

@IGreatlyDislikeJavascript
Copy link
Owner

IGreatlyDislikeJavascript commented Nov 6, 2019

I've made a PR that updates the README, adds a CHANGELOG, and a package.json, as well as adds a script to allow cutting a release

Fantastic, thank you. I don't know how you did all that, but it's really appreciated - I hope you have an automated tool and didn't need to do it all by hand :-) .

I'm going to merge your PR and mark it as v0.3.1 release now.

One question - I assume I'm still OK to not release this on npm, because we still have the question over the backdrop? Or if I mark it as a release, am I supposed to also release it on npm even knowing there's a potential impact because of the significant backdrop change?

edit another question - you've referenced a script to cut releases. Please could you explain how that's used?

@tszulc
Copy link
Contributor

tszulc commented Nov 6, 2019

I agree with waiting on releasing this for now, until a fix is found, and once that's added, I would bump the versioning to a MAJOR update (ie. v1.0.0), and continue from there.

Seem I forgot to add a link to the package in the PR: conventional-changelog/standard-version
These are the steps we should follow whenever there's a PR or branch that's going to be merged into the master branch:

  1. Select the Squash and merge option with a title and body that follows the Conventional Commits Specification
    • NOTE: If the PR/branch includes more than one feature/fix, and each commit uses a structured message, we can just Create a merge commit it in.
  2. When you're ready to release:
    1. git checkout master; git pull origin master
    2. npm run release
    3. git push --follow-tags origin master && npm publish (we can ignore the npm publish part, if we don't want to release to npm just yet)

So if we use the example of fixing the backdrop issue, we would:

  1. Select Squash and merge on the PR
  2. Add a title, something like fix: backdrop issue
  3. Maybe add something to the body with more details (if so desired)
  4. We need to switch to, and pull master on our machines
    • git checkout master; git pull origin master
  5. We then use the package to update the CHANGELOG, bump our npm package version, and our git tag to v1.0.0
    • npm run release --release-as 1.0.0
  6. We then need to push those local changes up to github, and publish the package to npm
    • git push --follow-tags origin master && npm publish

For most other cases we would do all of the above except that on Step 5 we would run npm run release instead.

Since you seem to have your GPG key set up, add the --sign or -s flag (ie. npm run release -- --sign)

@IGreatlyDislikeJavascript
Copy link
Owner

Thank you for taking the time to explain this to me.

I think now might be a good time to point out that I simply use the web interface of github. Sorry :-(

I've read through your example and the npm release documentation. I think I get it, but the exact effect of the process is a bit hazy. One question: why are your steps 5 & 6 in that order?

Step 5 creates a github release automatically, and step 6 (ignore && npm publish) pushes the local changes (my understanding).

But why don't we make the local changes (CHANGELOG updates etc), then push to github first as per step 6, and then make the release as per step 5, and then publish as per step 6?

The reason for the order is a little unclear to me.

I'm heading off on vacation after tomorrow so will try and get everything done before.

@tszulc
Copy link
Contributor

tszulc commented Nov 8, 2019

Since the package only makes the changes locally, we have to push those changes up to actually create a release.

  • So first we get the most recent version of master (git checkout master; git pull origin master), after we've merged the branch/PR, or added a commit.

  • Then we run the package (npm run release), to update the CHANGELOG, and it bumps the version in the package.json file, and creates a local tag. The version bump depends on the commit's title:

    Title Stage Rule Example version
    fix PATCH Increment the third digit 1.0.1
    feat MINOR Increment the middle digit and reset last digit to zero 1.1.0
    <type>! MAJOR Increment the first digit and reset middle and last digits to zero 2.0.0
  • Finally, we have to push these local changes up to github, with the local tag the package just created (git push --follow-tags origin master). After which we should also publish the version to npm (npm publish)

@IGreatlyDislikeJavascript
Copy link
Owner

Thanks for taking the time to explain this, it makes sense now - and I'm sure will make even more sense when I do it the first time.

As above I'll pause doing this until we've got the backdrop question fixed. I think ibastevan has already fixed it but it's not in a PR, and unfortunately I've run out of time to diff his fork before vacation (not so unfortunate for me!). Assuming all is well, I'll get everything done including these npm/release tasks when I get back later this month.

Thanks again.

@IGreatlyDislikeJavascript
Copy link
Owner

@tszulc thanks for your help with this, I think I've successfully published v0.3.2 to npm now. However I got a bunch of errors when attempting to npm run release . I don't know if this is a problem or not?

me@mybox:~/Code/bootstrap-tourist$ npm run release

> [email protected] release /home/me/bootstrap-tourist
> standard-version

sh: 1: standard-version: not found
npm ERR! file sh
npm ERR! code ELIFECYCLE
npm ERR! errno ENOENT
npm ERR! syscall spawn
npm ERR! [email protected] release: `standard-version`
npm ERR! spawn ENOENT
npm ERR!
npm ERR! Failed at the [email protected] release script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/me/.npm/_logs/2019-12-09T18_37_58_003Z-debug.log

@tszulc
Copy link
Contributor

tszulc commented Dec 10, 2019

@IGreatlyDislikeJavascript did you run npm install before? It doesn't seem like you have the standard-version package installed on your machine

@IGreatlyDislikeJavascript
Copy link
Owner

@tszulc no, I didn't do that. I'll do that in prep for the next major version release. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants