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

feat(cli): generate changelog from JSON context #784

Merged
merged 14 commits into from
Aug 23, 2024

Conversation

janbuchar
Copy link
Contributor

@janbuchar janbuchar commented Aug 4, 2024

Description

This implements generating a changelog from a pre-generated JSON context, which allows for more extensibility.

Motivation and Context

How Has This Been Tested?

As of now, only manual tests have been performed. I'm not sure what the best strategy for unit-testing this would be.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (no code change)
  • Refactor (refactoring production code)
  • Other

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
  • I have formatted the code with rustfmt.
  • I checked the lints with clippy.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@janbuchar janbuchar requested a review from orhun as a code owner August 4, 2024 20:25
Copy link

welcome bot commented Aug 4, 2024

Thanks for opening this pull request! Please check out our contributing guidelines! ⛰️

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 19.04762% with 34 lines in your changes missing coverage. Please review.

Project coverage is 39.42%. Comparing base (b8045e9) to head (3e7c395).
Report is 6 commits behind head on main.

Files Patch % Lines
git-cliff/src/lib.rs 0.00% 32 Missing ⚠️
git-cliff-core/src/changelog.rs 77.78% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #784      +/-   ##
==========================================
- Coverage   39.61%   39.42%   -0.18%     
==========================================
  Files          20       20              
  Lines        1606     1634      +28     
==========================================
+ Hits          636      644       +8     
- Misses        970      990      +20     
Flag Coverage Δ
unit-tests 39.42% <19.05%> (-0.18%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@janbuchar
Copy link
Contributor Author

This would be more useful if we added an optional free-form dictionary (e.g., additional_data) to both Release and Commit. In the current state, we may only change the existing context fields.

Copy link
Owner

@orhun orhun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn, this is nice! Thanks for going ahead and implementing it 🔥

This would be more useful if we added an optional free-form dictionary (e.g., additional_data) to both Release and Commit. In the current state, we may only change the existing context fields.

I definitely agree, would love to have that. We can use serde_json::Value for that I reckon.

A couple of things:

  • We should update the documentation about this. We can create a new page under /docs/usage. (I'm okay with adding it if you are not interested :))
  • Can you add a fixture test to verify this behavior?

@janbuchar
Copy link
Contributor Author

Damn, this is nice! Thanks for going ahead and implementing it 🔥

This would be more useful if we added an optional free-form dictionary (e.g., additional_data) to both Release and Commit. In the current state, we may only change the existing context fields.

I definitely agree, would love to have that. We can use serde_json::Value for that I reckon.

Great! Is extra an acceptable name? The originally proposed additional_data is also an OK option, but a bit long IMO.

  • We should update the documentation about this. We can create a new page under /docs/usage. (I'm okay with adding it if you are not interested :))

If you could just push a file with some kind of an outline here, I'll be glad to fill it in.

Absolutely, once we've settled on the extra attribute name.

@orhun
Copy link
Owner

orhun commented Aug 7, 2024

Yes, I think extra (or extra_data or custom_data) is a good name!

@janbuchar
Copy link
Contributor Author

janbuchar commented Aug 15, 2024

@orhun I added the extra attribute and a fixture test. However, it seems like there is a bug - the commit.extra field gets lost somewhere on its way into the template. I found that it is still present in changelog.rs#542 (line 530 in current main, I did a dbg!(&release) there), but it is not present inside the Template.render method. How can that happen?

@orhun
Copy link
Owner

orhun commented Aug 20, 2024

Ah, there is a custom Serialize implementation for Commit where you should add the new field :)

impl Serialize for Commit<'_> {

@janbuchar janbuchar requested a review from orhun August 22, 2024 20:58
@janbuchar
Copy link
Contributor Author

@orhun I added a simple docs page and everything except Docker automated builds passed - those are still running.

Copy link
Owner

@orhun orhun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hot stuff 🔥

I pushed some commits to improve this even further:

  • now supports GIT_CLIFF_CONTEXT environment variable
  • remote data is made available via calling add_remote_data on context
  • support stdin out of the box via checking is_terminal() (e.g. cat context.json | git-cliff)
  • updated docs

I think this opens so many doors as an extension point, so thanks a lot again! <3

@orhun
Copy link
Owner

orhun commented Aug 23, 2024

Oops, is_terminal() check seems to have broken the fixture tests, I will look into it later.

ref: 135938f

@janbuchar
Copy link
Contributor Author

Thanks for the review and for the improvements, I can't wait to use it!

@orhun
Copy link
Owner

orhun commented Aug 23, 2024

Will cook up a release pretty soon 🤞🏼

@orhun orhun merged commit 3b6156d into orhun:main Aug 23, 2024
59 of 60 checks passed
Copy link

welcome bot commented Aug 23, 2024

Congrats on merging your first pull request! ⛰️

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.

Support generating changelog from a JSON context
3 participants