Add new changelog subcommand #769
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
| // PatchYAML looks for the proper place to add the new revision in the changelog, | ||
| // trying to conserve original format and comments. | ||
| func PatchYAML(d []byte, patch Revision) ([]byte, error) { | ||
| var nodes []yaml.Node |
There was a problem hiding this comment.
I understand why you did it this way (generic YAML parsing), but maybe it's fair to depend on schema and unmarshal the changelog to the proper structure? I don't mind if we lose some comments in the meantime.
There was a problem hiding this comment.
I would prefer to change the files the minimum possible and maintain the current format, at the end these files are still maintained by humans.
I gave a try to yaml patch libraries for this, and they work well for simple changes as setting the version in the manifest, or appending elements, but I didn't manage to add the changelog entry on top. I can give this approach another try so we don't have to maintain this parsing code, but we can still keep the format and comments.
There was a problem hiding this comment.
I guess that it's a matter of personal preference, YAML patching vs strict model. I don't have any strong points for switching to mapping except for simpler implementation and less node-level processing.
BTW as an alternative approach, you can consider mapstr. It's up to you to decide which option seems less error-prone :)
There was a problem hiding this comment.
Yeah, this code is a bit too much for what it does, but I didn't find another reliable way to do it with the requisite (maybe self-imposed :) ) of maintaining the format as much as possible.
In any case I have tried to maintain the interfaces of the two exposed methods clean, and tests cover them, we can change their implementation if we get another idea.
| return nil, errors.Wrap(err, "unexpected manifest content") | ||
| } | ||
|
|
||
| setYamlMapValue(node.Content[0], "version", version) |
There was a problem hiding this comment.
Same concern as for the changelog. In this case, we already have the logic to read the package manifest.
|
FYI @marc-gr as your team may benefit from this pull request :) |
3 participants
Add a new subcommand with utilities for the changelog.
It includes an
addcommand that can be used to add changelog entries,this can be useful in automation.
For example the following command creates a new patch version including a change with a description, link and type:
If no
--nextor--versionflags are passed, change is included in current version.