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

Altering Types of changes #12

Closed
stropho opened this issue Feb 6, 2020 · 5 comments
Closed

Altering Types of changes #12

stropho opened this issue Feb 6, 2020 · 5 comments

Comments

@stropho
Copy link

stropho commented Feb 6, 2020

Great tool!

I understand it supports the Types of changes based on https://keepachangelog.com/en/1.0.0/
However, I think that a change of those "Types of changes" could be beneficial to many users. For example some could add a "Upgrade" type or anything else...
Well, it was discussed a while back here brightcove/kacl#6 But with no additional initiative it seems...

Is this something that would be considered?

Solution proposals:

  1. probably the easiest: don't change implementation. the Release class can be extended. Just cover it by a test and mention in documentation, so people can rely on it.
  2. based on point 1. - Have an Additional class ReleaseBase that could e.g. accept another parameter in constructor to define the types. Additionaly, ReleaseBase does not include methods like added, changed etc. Release class would extend ReleaseBase . It still doesn't introduce any breaking changes.
  3. Change the current Release class to accept optional parameter for types (in constructor). No way to add new methods for specific change types.
  4. Have an additional method in Release class setTypes. That just doesn't seem right
@oscarotero
Copy link
Owner

I think, for now, the proposal 1 is the best, because it's pretty easy to extend the changes with more types. Just note that currently there's a function for each type (added(), changed() ...) so I would not replace the current types, for backward compatibility, just add more different types).

Other problem I see is the parser, that needs a way to set a custom Release class to create the instance here: https://github.com/oscarotero/keep-a-changelog/blob/master/src/parser.js#L28

@stropho
Copy link
Author

stropho commented Feb 6, 2020

Good point. Haven't noticed that thing with the parser.

The only quick fix I can think of now is a bit of Dependency Injection:

function defaultReleaseCreator(title, description){ return new Realease(title, description)}
function parser(markdown, releaseCreator = defaultReleaseCreator) {
//...
}

@oscarotero
Copy link
Owner

I think that an object with options (so it can be extended in a future with more settings) is better. For example:

const options = {
    releaseCreator: (title, description) => new Release(title, description)
}

const changelog = parser(markdown, options);

If you want to work on a PR for this, I'd appreciate it very much.

@stropho
Copy link
Author

stropho commented Feb 6, 2020

I'm on it 😉

@stropho
Copy link
Author

stropho commented Feb 7, 2020

#13

@stropho stropho closed this as completed Feb 7, 2020
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

2 participants