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

release: false should be the lowest priority rule #377

Open
nweajoseph opened this issue Sep 15, 2022 · 2 comments
Open

release: false should be the lowest priority rule #377

nweajoseph opened this issue Sep 15, 2022 · 2 comments

Comments

@nweajoseph
Copy link

nweajoseph commented Sep 15, 2022

Consider what happens when we expand this test to apply scope:

test('Return "false" for release type ONLY if the matching rule has no higher "release"', (t) => {
  const commit = {type: 'fix', scope: 'frontend'};

  t.is(analyzeCommit([
    {type: 'fix', scope: 'frontend', release: 'patch'},
    {type: 'fix', scope: '*', release: false} // <----- this should NOT override 'patch'
    ], commit), 'patch');
});

so this ruleset says "release fixes as 'patch' for the frontend, but don't release any other fixes in any other scopes"

this test will fail. because of this line: https://github.com/semantic-release/commit-analyzer/blob/master/lib/compare-release-types.js#L11

RELEASE_TYPES doesn't include false or null. so the algorithm starts to analyze our commit using our ruleset, and correctly identifies it as a 'patch'. then it considers the next rule, which says release === false. false is not in RELEASE_TYPES, so compareReleaseTypes() returns true, and the release value for the commit is incorrectly set to false, instead of patch.

this is problematic because it violates the advertised behavior of the release configuration: a commit will always be given the highest possible release value available from the given ruleset. if we consider "no release" the lowest possible release value - as I think is intuitive - then any other release value should take precedence.

the current algorithm treats no release as a special case without priority. or rather, it implicitly treats no release as the highest possible priority of release type, which doesn't make sense. no release cannot be the biggest release.

the current behavior thus renders the use case I've alluded to impossible, so i'll spell that use case out more specifically.

if you have a repo with several scopes, then you might want rules that govern how some scopes get released, and you might want only some of those scopes to get released ever. for example, if you have a project like this:

project/
  - frontend
  - backend
  - shared-tools

you might want to say something like this:

"release any frontend or backend commits according to standard rules. fix:patch, feat:minor, break:major. but don't ever release any commits in shared-tools no matter what."

here's an example of what that ruleset might look like:

[
  {scope: 'frontend', type: 'fix', release: 'patch'},
  {scope: 'frontend', type: 'feat', release: 'minor'},
  {scope: 'frontend', type: 'breaking', release: 'major'},
  {scope: 'backend', type: 'fix', release: 'patch'},
  {scope: 'backend', type: 'feat', release: 'minor'},
  {scope: 'backend', type: 'breaking', release: 'major'},
  {scope: '*', type: '*', release: false}
]

but for the reasons explained so far, that ruleset will never do what you expect. it will make any release impossible.

the simplest fix and what i would recommend is adding false, and null to the end of the list of RELEASE_TYPES. this would be a breaking change for people who expect release: false|null to override any other possible release value. but that perspective doesn't make any sense, and renders reasonable workflows impossible.

@nweajoseph
Copy link
Author

PR: #378

@nweajoseph
Copy link
Author

nweajoseph commented Sep 15, 2022

Another use-case for this change is ignoring nonsense. For example, how would you express this policy as a rule:

frontend and backend are both valid scopes. treat them in the standard way. any other nonsense people might write for "scope" should be ignored. frontend and backend are the only "real" scopes

? without {scope: "*", release: false} i don't see any way to express that. the same example can be produced for type

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

1 participant