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

Improvement of conventional commit messages #53

Closed
exactlyaron opened this Issue Jun 14, 2018 · 11 comments

Comments

Projects
None yet
3 participants
@exactlyaron

exactlyaron commented Jun 14, 2018

Hello, (yes, me again)

I've done a bit of a search;

dependabot/dependabot-core#246
dependabot/dependabot-core#192

Because of the way things are going we're moving away from the monorepo approach and having things separate to make it easier for publishing and building.

So we're using semantic-release and commitizen with the cz-conventional-changelog which is based on Conventional Commits.

I'd like to request some enhancements. (Unless they're already in place and I'm missing them again)

Greenkeeper differentiates between dependancies & devDependancies. deps get a fix: and devDeps get a chore. The fix triggers a release. This is perfect for our a lot of our packages and in theory how we'll be building products. At least how I see it happening in my head any way. It would be nice if dependabot had a config to do something similar? 😉

I might be able to jig things with commit-analyzer if dependabot was doing a conventional message ... but it might end up being a bit messy.

There is a lot of love for dependabot at wmfs though and the support for Docker will come in handy very soon! 👍

Thanks,

@greysteil

This comment has been minimized.

Member

greysteil commented Jun 18, 2018

Thanks for the feedback!

On conventional commits, it looks like it was just a case of Dependabot recognising that your repo had switched over. I can see that wmfs/food-hygiene-blueprint#8 uses angular style.

On scoping, what's your use case for it? I removed it a few months ago because Angular themselves don't use it for package upgrades (Angular commits here).

On differentiating between deps and devDeps, I can see where you're coming from, but are you sure you want t release each time one of your deps gets a patch / minor release? If you're not pinning your dependencies, which for a library you probably shouldn't be (to increase the number of dependencies that can be shared by those downloading it), then a release isn't really necessary I don't think? It also feels a bit weird to mark performance enhancements in sub-dependencies as a fix (or feature additions as a feat, since it's not a feat in your library).

@zeke

This comment has been minimized.

zeke commented Jun 21, 2018

I'm also using semantic-release and would love for dependency update to be able to trigger releases.

@greysteil

This comment has been minimized.

Member

greysteil commented Jun 21, 2018

@zeke - do you have an idea of how exactly you would want that to work?

@zeke

This comment has been minimized.

zeke commented Jun 21, 2018

I think the way Greenkeeper does it works well. I think for runtime deps they do a fix: prefix and for dev deps they do chore:

I know configuration is kind of a last resort, but maybe a configurable prefix would be good?

Just to throw a wrench in things here, I'm working on a new project https://github.com/electron/algolia-indices which should ideally be (semantically) released when any dep is updated, regardless of whether it's a runtime dep or a dev dep.

@exactlyaron

This comment has been minimized.

exactlyaron commented Jun 22, 2018

On conventional commits, it looks like it was just a case of Dependabot recognising that your repo had switched over. I can see that wmfs/food-hygiene-blueprint#8 uses angular style.

Okay that makes sense, I've removed the old monorepo and setting up the new repos individually now.

On scoping, what's your use case for it? I removed it a few months ago because Angular themselves don't use it for package upgrades (Angular commits here).

It's just a nice thing isn't it really. We're trying to get into the habit of writing better commits and making this easier to read.

On differentiating between deps and devDeps, I can see where you're coming from, but are you sure you want t release each time one of your deps gets a patch / minor release? If you're not pinning your dependencies, which for a library you probably shouldn't be (to increase the number of dependencies that can be shared by those downloading it), then a release isn't really necessary I don't think? It also feels a bit weird to mark performance enhancements in sub-dependencies as a fix (or feature additions as a feat, since it's not a feat in your library).

-----> edited the below

Right bare with me here, (@jezhiggins could probably explain better than I can) but basically Tymly is very modular in it's make up, with plugins and blueprints and a core Tymly at it's heart. The plugins in particular use Tymly for testing. Blueprints have plugins as dependancies. It's like a web but not really a web.

The tymly-core product, which is arranged in the way you'd normally expect - package at the top with a tree of packages underneath it.

We then have a whole array of plugins to that product. They don't have a dependency on the core, they have a devDependency because it's used in the their tests. Any change in the core, even a minor fix, could have implications on the plugins. Therefore, any time one of those core packages changes, we want to re-release the plugins so that we can say "this plugin version X has been verified against core version Y".

I'm not starting to think the initial title of the request should be different.

@exactlyaron

This comment has been minimized.

exactlyaron commented Jun 22, 2018

This may interest you as well @zeke

I might be able to jig things with commit-analyzer if dependabot was doing a conventional message ... but it might end up being a bit messy.

I've just had a go with commit-analyzer plugin and it's worked rather nicely and simply as well.

"release": {
    "analyzeCommits": {
      "preset": "angular",
      "releaseRules": [
        {"type": "build", "release": "minor"}
      ]
    }
  }

So when dependabot comes a long with a commit and it's merged the commit message having 'build:' will trigger a minor release. Party time! 🎉🎉

If dependencies and devDependancies were differentiated by Dependabot? Similar to Greenkeeper, then we could use the plugin to give devDependancies a patch release etc if it's updated as a chore for example.

But Dependabot (when we pay for the private repo access 💰 ) is going to work super for how we're building Tymly products though, basically everything gets installed as defined by a package.json, node_modules is thrown into a Docker image and that's pushed ready to be used.

So with the greater control of bumping we'd be able to define specific builds for show and tells or tested by customers then they only get bumped and build at specific days of the week.

@greysteil

This comment has been minimized.

Member

greysteil commented Jun 22, 2018

Interesting. I can see an argument for using the scope to differentiate between dev and production dependencies - that would allow you to customise the logic on your side (hopefully?), which I'm 100% onboard with.

What I don't like is changing the commit type itself to fix or whatever for production dependencies. I know Greenkeeper does it, but it's not accurate - a lot of those changes will be build changes, and naming them fix for the benefit of people using automated release tools, at the expense of a misleading name for everyone whose not, doesn't feel right to me.

How about we add in scopes, and go for build(deps): ... and build(devDeps): ...?

@exactlyaron

This comment has been minimized.

exactlyaron commented Jun 22, 2018

Yeah that's understandable in the world of conventional commits. Scoping would be perfect actually as that can be defined in the release rules of the commit analyzer.

There is no emoji to show my appreciation for this @greysteil ! 👍 Thanks a lot!

@greysteil

This comment has been minimized.

Member

greysteil commented Jun 22, 2018

Haha, awesome, I'll get it shipped today 🙂

@greysteil

This comment has been minimized.

Member

greysteil commented Jun 22, 2018

:shipit: Shipped! dependabot/dependabot-core@ef1b321.

If there's a backlash from people who know more than me about scopes then I may have to revert, but I think it should be fine, and is definitely worth trying. Thanks for the feedback @exactlyaron!

@greysteil greysteil closed this Jun 22, 2018

@zeke

This comment has been minimized.

zeke commented Jun 24, 2018

Thanks, @greysteil 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment