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

Warnings when generating CHANGELOG.md #195

Open
gian1200 opened this issue Oct 12, 2024 · 7 comments
Open

Warnings when generating CHANGELOG.md #195

gian1200 opened this issue Oct 12, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@gian1200
Copy link

gian1200 commented Oct 12, 2024

Describe the bug

2 warnings:

  • When running commit-and-tag-version on Windows (and having default git configuration), file is generated with LF instead of CRLF.
  • According to markdownlint, generated CHANGELOG.md has an extra (unnecessary) break line. One line before each commit type header is enough. No need to put 2.

Current behavior

npx commit-and-tag-version --first-release
× skip version bump on first release
√ outputting changes to CHANGELOG.md
√ committing CHANGELOG.md
warning: in the working copy of 'CHANGELOG.md', LF will be replaced by CRLF the next time Git touches it

warning: in the working copy of 'CHANGELOG.md', LF will be replaced by CRLF the next time Git touches it
warning: in the working copy of 'CHANGELOG.md', LF will be replaced by CRLF the next time Git touches it
...

image
image

Expected behavior
No warnings

Environment

  • commit-and-tag-version version(s): v12.5.0
  • Node/npm version: Node 20.17.0/npm 10.8.2
  • OS: Windows 11

Possible Solution

  • Read git configuration (git config --get core.autocrlf) and generate the file accordingly

  • Remove the extra line from generated file

Additional context

@gian1200 gian1200 added the bug Something isn't working label Oct 12, 2024
@gian1200
Copy link
Author

gian1200 commented Oct 12, 2024

Update:

After reading the code, it sems that both issues come from conventional-changelog.
Also, at least one is already fixed on conventional-changelog-conventionalcommits v8.0.0

Currently commit-and-tag-version references an old version

"dependencies": {
"chalk": "^2.4.2",
"conventional-changelog": "4.0.0",
"conventional-changelog-config-spec": "2.1.0",
"conventional-changelog-conventionalcommits": "6.1.0",
"conventional-recommended-bump": "7.0.1",

@TimothyJones
Copy link
Member

That's right! The linked issue is the right place to report this. I'll leave this issue open, pending their fix (since it will mean we'll need to bump the version here).

On the fixed half of this issue - I'm definitely keen to get in the newer versions of the conventional-changelog libs - I haven't yet because it'll require a close read of the breaking changes. I'm happy to merge it in and release a breaking bump here, but I want to make sure our changelog is right.

I'm not sure how much work it'll be - it's possible that the very latest will mean we'll need a conversion to ESM (which is worth doing anyway).

@TimothyJones
Copy link
Member

If anyone has the capacity to look at migrating to the later versions, I'd very happily accept PRs. Otherwise I'll get to it when I can.

@gian1200
Copy link
Author

gian1200 commented Oct 17, 2024

I might be able to do it in the next few days/weeks. I also notice that there are other dependencies that need some love, too (e.g. ESLint).

My only question would be regarding contribution guidelines (I didn't see any CONTRIBUTING.md). For instance:

  • to not include package.lock.json (for security reasons), and only put package.json in PRs; being the responsibility of maintainers to complete the final step (run npm install)
  • to not mix multiple changes on a single PR
  • etc.

@TimothyJones
Copy link
Member

Huh, I thought we had one, but it seems not.

I agree with both of those. Generally it's also:

  • If adding / changing a feature please ensure there are tests for the new behaviour
  • If there are user facing changes / new features, please ensure the readme is updated
  • Please title the PR with conventional commits, as PRs are squash merged - and of course, these become the changelog, so it's best for fix / feat commits to be titled assuming they're read by users

@gian1200
Copy link
Author

gian1200 commented Oct 21, 2024

I tried to upgrade it, but the ESM migration is required on latest versions of conventional-changelog packages.

I also tried to upgrade them to latest version compatible with CommonJS. However, I found some issues with presets while running tests. I wasn't able to find a fix, so I took the opportunity to work on the following:

These should:

  • ease migration to ESM
  • get feedback regarding PRs on the repo

@gian1200
Copy link
Author

UPDATE

Preset issues are due to conventional-changelog/conventional-changelog#1308

So the only way to upgrade conventional-changelog is to move to ESM or wait for the fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants