-
Notifications
You must be signed in to change notification settings - Fork 85
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
docs: Update CHANGELOG.md through version 0.3.0 #63
Conversation
This comment has been minimized.
This comment has been minimized.
👍 (sorry to say, since these updates to CHANGELOG.md do look like a step in the right direction) |
Just realized that this is a PR, not an issue. My plan is to continue on this. So I'm just marking it as a draft for now. |
Only referencing xmldom#62 for further discussions.
From my point of view this is done, as far as already published versions are concerned. I would suggest to introduce (something like) https://github.com/CookPete/auto-changelog to make it easy to update the CHANGELOG in the future. This is what it could look like with minimal configuration and manual tweaks: I can take care of it directly after this PR, so it can already be applied when releasing 0.4.0 |
I am reviewing this now, my apologies for the delay. +1 for automating the change log, with some questions coming up |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some nits & minor suggestions. In general:
- I would favor keeping present-tense verbs
- I would avoid parens in the version section headers, to keep difference from generated anchors as minimal as possible (I hope we will never have to deal with xmldom vs xmldom-alpha packages again)
I have never seen
I would probably favor putting this into a PR, which should be easier to review and comment on. As I said before, I generally have not listed devDependencies and test updates in the change logs, open for discussion. |
Co-authored-by: Chris Brody <chris.brody+brodybits@gmail.com>
Thx for the review. I completely removed the notion of xmldom vs xmldom-alpha packages from the headers and added that information just below the publish date. I converted everything (including older entries) to present tense. Also made sure the order is always consistent without any empty lines between them
For versions |
Yes, I just didn't want to invest to much time into it, since I will need to rebase it etc and some details still have to be discussed. I also think it shouldn't block the release of 0.4.0 but it would be nice to already include. I think the tool just helps to list everything and then it's up to the person publishing the package to decide what information to keep. |
Please feel free to merge when you think it's ready. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the liberty to rename this PR (2x) and left what are mostly nits.
One more nit: It looks like we are missing information before version 0.1.18.
I discovered that there is a version 0.1.17 on npm but not in the git history. This would probably be around 2013 or 2014. They may have published 0.1.17 from a special branch or tag in the original repository.
I would probably favor adding a note that details are unknown for versions before 0.1.18.
@karfau I think you should be able to merge this no problem, since it would not be part of a release tag (as a squash commit, please).
CHANGELOG.md
Outdated
|
||
## Maintainer changes | ||
|
||
After the last commit to the original repository <https://github.com/jindw/xmldom> on the 9th of May 2017, the first commit to this fork is from the 19th of December 2019. [The fork has been announced in the original repository on the 2nd of March 2020.](https://github.com/jindw/xmldom/issues/259) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Subjective: I am torn at this point whether to refer to this repo as a "fork" or as a new project location.
Assuming we continue to publish into the same xmldom
package on npm, I would not really favor calling this a "fork".
OTOH if we would start publishing this as multiple packages in the @xmldom
namespace on npm, which I had already reserved as an org (see issue #54), then I think "fork" would be appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about replacing this fork
with<https://github.com/xmldom/xmldom>
so it stays correct even if it should ever move somewhere else. (Already applied that.)
I think "the announcement of the fork" is still the correct wording for describing what happened back then.
|
||
The new maintainers do not plan to continue publishing the `xmldom-alpha` package. | ||
|
||
The new maintainers did not invest time to understand changes that led to the last `xmldom` version [`0.1.27`](#0127) published by the original maintainer, but consider it the basis for their work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
The new maintainers did not invest time to understand changes that led to the last `xmldom` version [`0.1.27`](#0127) published by the original maintainer, but consider it the basis for their work. | |
The new maintainer did not invest time to understand changes that led to the last `xmldom` version [`0.1.27`](#0127) published by the original maintainer, but consider it the basis for their work. |
the "did not invest time" part is very direct, and unfortunately accurate (perfectly accurate) as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or to "turn the knife" on me:
The new maintainers did not invest time to understand changes that led to the last `xmldom` version [`0.1.27`](#0127) published by the original maintainer, but consider it the basis for their work. | |
The new maintainer @brodybits did not invest time to understand changes that led to the last `xmldom` version [`0.1.27`](#0127) published by the original maintainer, but consider it the basis for their work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well not anybody of the "new maintainers" (which I consider to be the new xmldom org + contributors) took the time to do that, So I would leave the sentence as is.
And in case anybody takes the time, the sentence can be removed.
[...]
That makes sense to me. Maybe a draft PR, that I could use as a starting point for the release of 0.4.0, would make sense. Or maybe not. It may be ideal to set this up to make npm releases from GitHub actions, someday in the future. |
Co-authored-by: Chris Brody <chris.brody+brodybits@gmail.com>
Only referencing #62 for further discussions.