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

Port ChangeLogHelper to Fake.Core.Changelog #1876

Merged
merged 2 commits into from
Apr 22, 2018

Conversation

magicmonty
Copy link
Contributor

I just ported my ChangeLogHelper to FAKE 5 and made a new Package Fake.Core.Changelog for it.
I also fixed some bugs I encountered while using it (I'm the original author of the ChangeLogHelper)

@matthid
Copy link
Member

matthid commented Apr 20, 2018

I'm in principle OK with adding this, but first I'd like to ask why we cannot unify with Fake.Core.ReleaseNotes. What is this doing differently?

@kblohm
Copy link
Contributor

kblohm commented Apr 20, 2018

It would also be nice to be able to configure what indicates an added item or a changed item etc. That would be useful if you write your changelogs in another language. The same goes for the kind of header u use. Just today i could not use the releasenotes module because we format our changelogs with h1 headers and not h2.
Just an idea if you have time, otherwise i might do that later.

@magicmonty
Copy link
Contributor Author

The format is a kind of fixed format which is specified at https://keepachangelog.com/en/1.0.0/

So the supported ReleaseNotes have a much simpler format than this one.
In theory I'm fine with merging this with Fake.Core.ReleaseNotes but to distinguish the formats I would prefer to leave it in its own module/namespace so we could use Changelog.load instead of ReleaseNotes.loadChangelog what seems to be against the new API guidelines IMHO.

@kblohm how should this look like? Do you want a ChangelogParams type here, which can be filled with different headers? What would be the use case here? I could only imagine translation. Also there is also a Custom ChangelogEntry, which contains a custom header and its description.

@magicmonty
Copy link
Contributor Author

@matthid After some thoughts I think you are right. Conceptually belongs this to the Fake.Core.ReleaseNotes package as it just handles a different format.

I moved the Changelog.fs into the Fake.Core.ReleaseNotes package, but I left the Fake.Core.Changelog module by its own, so one can more easily decide which format one wants to choose.

BTW Where can I add some documentation for the FAKE website for this module?

@matthid
Copy link
Member

matthid commented Apr 21, 2018

@magicmonty I was more asking about what is actually different between the formats, couldn't we detect both and handle them via the same api?

@kblohm
Copy link
Contributor

kblohm commented Apr 21, 2018

@magicmonty Yes i did mean for translation. Maybe that is not too important but i could not use the module right now because our changelogs are in german.

@magicmonty
Copy link
Contributor Author

@matthid The format is much more detailed than the already supported release notes as it subsums the "release notes" in different categories. So a use in the same API is IMHO not possible (without losing information). Also the Changelog module is not only able to load and parse a Changelog but also to save it back and to promote an unreleased section to a new version.

@matthid matthid changed the base branch from master to rc_6 April 22, 2018 12:55
@matthid
Copy link
Member

matthid commented Apr 22, 2018

Thanks for looking into that :)

@matthid matthid merged commit b194ef2 into fsprojects:rc_6 Apr 22, 2018
@magicmonty magicmonty deleted the keepachangelog branch April 22, 2018 14:47
@magicmonty
Copy link
Contributor Author

Awesome, thanks

@matthid matthid mentioned this pull request Apr 22, 2018
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

Successfully merging this pull request may close these issues.

3 participants