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

Permit refactor/docs to be versionable #30

Open
rpocklin opened this issue Apr 7, 2017 · 5 comments
Open

Permit refactor/docs to be versionable #30

rpocklin opened this issue Apr 7, 2017 · 5 comments

Comments

@rpocklin
Copy link

rpocklin commented Apr 7, 2017

Currently, adding a refactor commit will not allow for a release. I don't like this behaviour, as it makes the assumption there are no bugs or other benefits in the refactor, which may be true, but you may need or want to bump the version.

This should be customisable so that the user can release a version even though it only has a 'refactor' commit in it.

I don't think these commits should make it into the changelog, as they are not real features.

@uglow
Copy link
Collaborator

uglow commented Apr 7, 2017

Hi @rpocklin , the use of fix|feat to trigger builds is a pretty core concept to the "conventional commits" convention. It's baked into most of the semantic-release tooling that I've come across. If this is a problem for you, there may be another convention which suits your workflow better?

While I understand that a refactor or docs may produce benefits, I would argue that if those benefits are significant to the consumers of your component then you should use "feat" to describe the change. So while you produced the benefits via the mechanism of refactoring or changing docs, a significant benefit (feature) was the end result. If the result of refactoring was of smaller significance to a consumer, then it would rightfully be deemed a refactor (or docs, as the case may be).

@rpocklin
Copy link
Author

rpocklin commented Apr 7, 2017

If you look through the issues others have raised in semver and semantic-release repos this has been raised quite a few times in agreement with my P.O.V. Unfortunately the official comment is to customise semantic-release:

semver/semver#146
https://github.com/jonathanong/ferver/issues/3
semver/semver#215
semantic-release/commit-analyzer#13
semantic-release/commit-analyzer#13 (comment)
semantic-release/commit-analyzer#3
semantic-release/semantic-release#261

This sums up the defaults I am suggesting:
https://github.com/BublTechnology/customizable-commit-analyzer

semantic-release use a version of commit-analyzer which is very outdated (last commit 2 years ago) and there have been PRs in regard to fixing this but nothing merged. This repo re-implements that same logic but doesn't mean it is correct.

Semver itself advocates releasing on each commit - (see https://github.com/semantic-release/semantic-release#is-it-really-a-good-idea-to-release-on-every-push)

Also consider the fact that it is quite possible a refactor can introduce bugs, even if the developer doesn't mark it as a BREAKING CHANGE.

Imagine 10 refactor commits are merged, then a single fix commit is merged, with a patch release to follow, which now contains 10 latent refactor commits shipping with it.

Here is a PR I wish was merged into commit-analyzer which would solve my problem:
semantic-release/commit-analyzer#4

Perhaps, in my case all we need is a process to do a manual patch bump the version, but that kind of defeats the purpose!

@uglow
Copy link
Collaborator

uglow commented Apr 7, 2017

Thanks for sending through supporting doc. I'm not against allowing people to use their own configuration, but I do think the current defaults should remain the current defaults. In https://github.com/BublTechnology/customizable-commit-analyzer, the defaults have been changed. This would be a breaking change for this module if we adopted it.

Semver is a statement of intent, backed up with tests, to give a consumer (and the producer) a level of confidence that the version number reflects the kind of changes made to the software.

I happen to like the fact that I can change documentation and code formatting AND do some minor refactoring without the version changing. I write tests. My tests are not perfect, but they give me a level of confidence that I'll know when I've broken something before I push my code. But if I didn't have great tests, or lacked confidence in them, then perhaps I would rather rely on semver to bump every time I push, "just in case".

Personally, I don't think semver should be used in this way - as a safety net in case I've broken the code. If we take "the possibility of producing bugs" argument to the extreme, why not configure semver to change the MAJOR version everytime, "just in case"? What this would do is break the meaning of semver for your consumers, as in most cases the version number would be "crying "Wolf!"".


If can conceive a method of supporting your needs without changing this module's defaults, I think that would be doable. @leonardoanalista thoughts?

@rpocklin
Copy link
Author

rpocklin commented Apr 8, 2017

Yep i'd be happy with that - it should be easy to push out to a config file. You could even define if it is changelog-worthy or not at the same time ie.

{
  commits: {
    refactor: {changelog: true, bump: 'patch'},
    fix: {...},
    feature: {...}
  }

@leonardoanalista
Copy link
Owner

We need to separate two aspects and consequences of a code modification.

  1. Software release
    A refactoring which doesn't add any value can be fully tested but may not be released. Reason: hard to explain to business a code change that does not add any value, does not improve performance.
    Refactoring they had a breaking change or possible braking change defies the purpose of semver and uses the mechanism as a safety net or CYA statement. Angular team makes use of this. I don't mind possible breaking chances I have written a few.
    If you have 10 commits, Maybe one of them can be interpreted as "perf" performance improvements, which triggers a release.
    Commmit type docs is a questionable one. I can't see nececity for a release or new version but can be interpreted as both ways.

  2. Chagelog contents
    This is a way to communicate inportant chances to your audicence. Usually the team, not necessarily tech people. At the moment you can specify your own changelog generator Adler documentation on readme.md

Corp-semantic-release API has been implemented similar to semantic-release. Yesterday I saw a twitter message asking for people to help with semantic-release. This may explain pull requests not being merged.

In summary, I m ok if refactor triggers a release. Imagine this scenario:

  • update version of angular from 1.5 to 1.6. This may course a refactoring to address new framework changes. However there is not feature added to consumers. In this case we still want to release but with the current implementation it wouldn't happen.
    I agree with breat regarding to default behavior. Any of this customizations could be added as opt-in options.

Leonardo

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