miniscruff/changie

Allow `changie batch auto` behavior while having kind configs with empty AutoLevel

adlotsof opened this issue · 19 comments

adlotsof commented

  1. When configuring changie kinds without AutoLevel (e.g. we do not want to release even a patch if we just add a test), changie batch auto will currently fail, even if other changes with kind config are present.

  2. A "major" auto level might prevent changie from failing, but this is not garanteed, depending on the order changie is reading the change files. (e.g. one change file with major AutoLevel while another change file with a kind without AutoLevel)

I am preparing a PR to address the first point

miniscruff commented

I am not sure I fully understand the use case for number one. Are you saying you have a change, that you want listed in the release, that does not count as any version bump?

Your example of a test is odd to me because to me, that wouldn't include a change fragment at all.

This would be similar to the chore label from other tools, these are things that you do that won't affect or be included in a release.

Such as updating CI, adding docs, fixing typos that sort of thing.

adlotsof commented

Yes, exactly. So having a .changie.yaml like so:

  - label: Feature
    description: new feature
    auto: minor
  - label: Chore
    key: Chore
    description: Minor tasks

If i have change files with both kind types, at the moment i will have an error. Error: no unreleased changes found for automatic bumping I would like to have a minor version bump.

adlotsof commented

The PR is introducing a flag to get the desired behavior (HighestAutoLevel does not error out if one of the change files is a kind withour an AutoLevel)

miniscruff commented

The design of changie suggests you do not include any chores in the release notes as they would not be user affecting changes. This is similar to the discussions around dependency updates.

That is to say, a change fragment indicates a user would be affected in some way and releasing would update some value in the version.

Supporting non-change changes feels outside scope unfortunately. But, maybe there is something you are doing with chores that require them to be part of the changelog.

Edit: I said design of changie but this is more like how semver works as a whole.

adlotsof commented

Essentially I want to have changes that I want to track in the changelog but that won't warrant a version bump. This is not server - compatible behavior - but will allow me to have semver-compatible behavior in an environment with multiple repos forming a version.

Not every change is visible to every user or affecting every user. So controlling visibility of the changes to whom they might affect is the goal.

I understand it's a super specific need

miniscruff commented

I am just not sure it adds any benefit to just assigning chores to patch updates and just waiting for another non chore change to come in before publishing?

adlotsof commented

Yeah the benefit for me is to use the change entries to determine the version to be published.
So changie next auto should determine the next tag, changie batch auto updates the changelog. This works super good, until i have changes i want to be described but not update the version.

I am just not sure it adds any benefit to just assigning chores to patch updates and just waiting for another non chore change to come in before publishing?

I could do as you describe and filter the changes before running changie, checking if there is a change entry that should trigger a version bump and only call changie if its needed.

Since the tool you created is really great in what it does and the modifications i did are really minor (and require a flag set to have effect), using changie seems easier than working around this limitation by pre-filtering the change entries. The behavior is optional: Using the AllowSkippedEntries flag (probably suboptimal naming and description) allows changie to determine the next version with kinds withouth auto, while setting this flag allows changelog entries that do not bump a version.

I could think of multiple ways how someone could use this flag even more creative e.g. automatically adding change fragments describing metadata like the PR#, the author, and such things while not adding them to the changes manually set by a user. There is probably more use cases for kindConfigs with empty AutoLevel that do not conflict with semver rules as much as my use case.

miniscruff commented

Hmm let me think about this, I am still not sold but maybe we add a preview label to whatever config option we add we can get feedback.

miniscruff commented

OK, I think, since I have had sort of similar requests, this feature is fine but I am wondering if we can just drop the error if auto is empty. Such that, an empty auto config is just effectively skipped. However, we should still error if you ONLY have empty auto configs. I think. Still not entirely sure on some of these edge cases.

I am also wondering if we can mark this change as something in preview or not, it feels like it may be a backwards incompatible change but not? Like, it used to fail but now is fine? Would that be a breaking change? I am not sure it needs to be an opt-in change via a config flag as its rather specific anyway. I am not sure if anyone is relying on the error in there use case or if it was an edge case that doesn't need to exist.

Open to thoughts of course.

adlotsof commented

I introduced the configuration flag AllowSkippedEntries to keep the behavior backwards compatible while still allow to skip.
The behavior with the flag not set should match the existing behavior.
The behavior with the flag set to true should be:

  • if a change file without auto: AND at least another change with auto: patch or higher, bump version to the highest auto:
  • if only change files without auto, throw an error
    So the configuration flag makes the new behavior optional, keeps the existing api. Its optional to deprecate the existing api in favor of the new behavior (error only if only empty auto: changes), but the way i would ask devs to use changie and feature flags in the project i want to implement this behavior: use a feature flag to enable the new api, announce old api deprecated and if there is no complaints, drop the old api.
miniscruff commented

Right, I think the configuration is fine as a temporary "hey, for now you have to opt-in to this behavior" but will be the default in say 3 months or something. And the reason I am ok with that is, in order for the existing behavior to be triggered you almost needed to do something by accident. Such as, calling changie batch auto without actually configuring auto mode. This will still fail if you didn't configure any auto modes. But in the event that you intentionally went out of your way to make only one or some of your kinds auto mode we just let it slide.

To be a little more explicit the following configs are currently valid:

  1. You do not use auto mode and have not configured any auto modes on any kinds, trying to batch auto will result in an error
  2. You have configured auto mode on all kinds, using batch auto will work

These two requirements are still maintained with or without the flag.

However, we are adding additional options of:

  1. You want auto mode, but one or more kinds are non-user changes you just want included without an explicit bump ( such as chores ). You can keep the auto value as empty which will no longer break if a change is found.
  2. If your release would only contain non-user changes, you get an error ( cause it wouldn't bump anything )
  3. If your release has a user change, we use that changes bump as normal

This is all just me agreeing with you btw, just being extra specific so that we can include these sorts of notes in the docs for auto mode. I believe this is basically what you already coded but since you created the issue and PR basically at the same time I wanted to make sure we were clear on the goal before giving my feedback on the PR.

miniscruff commented

This is also duplicated here #522 which I did decline to do, but if two completely separate people are asking maybe there is something I am missing.

cc @sheldonhull

adlotsof commented
  1. You do not use auto mode and have not configured auto modes on all kinds, trying to batch auto might result in an error

There is still this edge case in the existing behavior (and the reason why i did not open this issue earlier). Depending on the order of change files parsed by changie, if you have a auto:major change parsed before a change without auto:, you will have a major version bump.

Other than that i agree with your summary

miniscruff commented

oh right yea, cause I early returned on finding a major so it wouldn't fail in that case.

Hmm, I am still so torn on this, in my eyes this still feels slightly out of scope for changie. It is like the edge case for standard commits chores that are just parsed out but is being asked to be put in. But it's also not like, a huge amount of code changes to support and it seems to keep coming up.

Going to see if anyone else chimes in, I know sheldon did ask for this exact thing already.

sheldonhull commented

One thing I’ve realized lately is that the dependency updates actually do matter for the versioning. Let’s say renovate or dependabot provide a vulnerability fix that gets merged. I need the version to increment to actually publish this.

Innoway, I know have an extra step where I need to go add a change the entry manually to increment. With a protected branch this also can result in the need to run a PR and all status checks unless I have status check rules filtered down.

However, do I actually need to put this in a release note? It’s a bit subjective perhaps. Mostly not except this drives changie so currently I would need to.

I’ll also see that a lot of my work has been related to the developer audience in repos. Even if it’s not “customer facing” some of the changes relate to CI or build automation that has to trigger a release to make sure it works. Right now I just bump the patch version and designate some type of changes as related to CICD.

I don’t have an immediate proposal, but I will say these are some of the cases I’ve been working through to find the right balance with the changie approach, and still do more continual delivery.

miniscruff commented

Very insightful stuff sheldon. Would you say there is a missing feature here or it works as intended?

Testing CI/CD changes via releases is a weird thing for sure, but I would probably say that is true for any release notes tool. I would say if a downstream vulnerability was patched then updating that would count as a patch to your tool as well imo. Does it need a call out in your changelog, I think so, especially if you have a CVE link.

One thing I have thought about for a while is adding a github action bot thing that will let you create change fragments directly in PR comments. That might help some annoyances of having to make extra PRs for just changie fragments.

Like imagine if a comment like:
/changie new -k Fixed -b "upgrade curl to fix vulnerability" --custom Issue=${PR}
/changie batch auto
/changie merge

And the action would run all the commands for you, this would be useful for like, dependabot/renovate bot PRs that should be released.

I wonder if I can use the changie-action + github action to parse comments as commands... hmmm

sheldonhull commented
  • I have to stay github agnostic, as I mix github + azure repos. So for me, "PR ops" commands aren't useful.
  • I don't want to call out dependencies in detail. The PR can serve as a link for the CVE being patched. I say this because I regularly update with Renovate, and it's just wasting time for me to have to add that each time. Not only that but Renovate will block any updates once it gets updated by someone else. This means I'd have to merge the dependency and then do another PR each time for a change entry for it, just doesn't make sense.
  • I do like the PR ops idea, but I think changie should stick to being CLI driven. You could wrap those workflows up in github or mage tasks perhaps, but I wouldn't be able to leverage that personally. I drive all my automation as much as possible from mage (go tasks) instead of the logic being in the actions. So I actually don't get a lot of benefit from relying on github actions anyway. Example workflow helper codified bump

good discussion :-)

miniscruff commented

oh yea, I should of remembered that, you mentioned it before.

Ok what about this @adlotsof what if, as an experimental thing, the value of "none" for an auto was the indicator that its ok to skip this change. Since its not the zero value of the config we know you manually set this value. Saves the config and it will be much easier to drop/change the value of if it doesn't end up working out.

adlotsof commented

Implemented this in the pr