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

(GH-217) Template based release notes creation #283

Merged
merged 6 commits into from
Sep 4, 2020
Merged

(GH-217) Template based release notes creation #283

merged 6 commits into from
Sep 4, 2020

Conversation

akordowski
Copy link
Contributor

@akordowski akordowski commented Aug 26, 2020

Description

This PR addresses #217.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2020

Codecov Report

Merging #283 into develop will increase coverage by 1.01%.
The diff coverage is 87.50%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #283      +/-   ##
===========================================
+ Coverage    77.34%   78.36%   +1.01%     
===========================================
  Files           54       54              
  Lines         1130     1137       +7     
  Branches       135      131       -4     
===========================================
+ Hits           874      891      +17     
+ Misses         235      225      -10     
  Partials        21       21              
Impacted Files Coverage Δ
...GitReleaseManager.Core/Options/CreateSubOptions.cs 85.71% <0.00%> (-14.29%) ⬇️
...seManager.Core/ReleaseNotes/ReleaseNotesBuilder.cs 90.47% <86.04%> (+10.47%) ⬆️
...c/GitReleaseManager.Core/Commands/CreateCommand.cs 100.00% <100.00%> (ø)
src/GitReleaseManager.Core/VcsService.cs 72.41% <100.00%> (+1.49%) ⬆️
...anager.Core/Configuration/ConfigurationProvider.cs 45.28% <0.00%> (-1.53%) ⬇️
...ReleaseManager.Core/Extensions/StringExtensions.cs 100.00% <0.00%> (ø)
...easeManager.Core/Configuration/ConfigSerializer.cs 90.00% <0.00%> (+1.53%) ⬆️
...eManager.Core/ReleaseNotes/ReleaseNotesExporter.cs 69.56% <0.00%> (+4.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4010dc5...9ce4a06. Read the comment docs.

@akordowski
Copy link
Contributor Author

@gep13 @AdmiringWorm In the first step I added the Scriban library to have a template based release notes creation. If you agree with the changes, I would add in next step a -template option to the Create command and update the docs.

A question is how should it be handeled when the -template is set but the file is not found?

  • Show a warning and use the default template
  • Or throw an exception?

@gep13
Copy link
Member

gep13 commented Aug 31, 2020

@akordowski I would suggest that if the user has asked to use a template, and that template file doesn't exist, then this should be treated as an error, and we should throw an exception, indicating that the required file couldn't be found.

@gep13
Copy link
Member

gep13 commented Aug 31, 2020

@akordowski if the template file is empty, then this to me is also an exception, and we should throw an error.

@akordowski akordowski marked this pull request as ready for review September 3, 2020 12:40
@akordowski
Copy link
Contributor Author

@gep13 I made the changes as discussed. The PR is ready for review.

Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

One minor change...

docs/input/docs/commands/create.md Outdated Show resolved Hide resolved
@gep13
Copy link
Member

gep13 commented Sep 3, 2020

@akordowski this is looking really good!

One thing though, would it be possible for you to squash some of your commits down into a set of logical commits? We don't really need additional commits like "Fixed Unit Test". Thanks

@akordowski
Copy link
Contributor Author

@gep13 I made the changes as requested.

Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13
Copy link
Member

gep13 commented Sep 4, 2020

I believe the build failure here, after the rebase, isn't related, it is an issue with codecov.

@gep13 gep13 merged commit a41561e into GitTools:develop Sep 4, 2020
@gep13
Copy link
Member

gep13 commented Sep 4, 2020

@akordowski thanks again for getting this done, really appreciate it!

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