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

[Add] Include severity changelog template for MD #660

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

DevMrRober
Copy link

@DevMrRober DevMrRober commented Feb 26, 2025

Summary

This PR introduces a severity classification to the API changelog markdown.

Changes made

  • Updated GroupChanges function in formatters/changes_by_endpoint.go
  • Updated changelog.md template in formatters/templates/changelog.md.

Reason for change

  • The update was made to improve the clarity of the changelog and better structure the changes based on severity and impact.
  • This ensures that the breaking changes are highlighted and that the changelog displays the changes in a more consistent format.

Tests

  • All existing tests have been run to ensure that no functionality has been affected by this change.
  • Requests are added correctly without being removed, and existing functionality remains intact.

Reviewers

  • Please review the changes and provide feedback on the removal of the feature.

Add: Change error level on GroupChanges
@DevMrRober DevMrRober changed the title Changelog Generation: Add severity and change template [Add] Include severity and change template Feb 26, 2025
@DevMrRober DevMrRober changed the title [Add] Include severity and change template [Add] Include severity changelog template for MD Feb 26, 2025
Add consideration for the changelog as well.
Copy link

codecov bot commented Feb 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.52%. Comparing base (04dad02) to head (948f345).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #660   +/-   ##
=======================================
  Coverage   87.52%   87.52%           
=======================================
  Files         237      237           
  Lines       13726    13728    +2     
=======================================
+ Hits        12014    12016    +2     
  Misses       1298     1298           
  Partials      414      414           
Flag Coverage Δ
unittests 87.52% <100.00%> (+<0.01%) ⬆️

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.

@reuvenharrison
Copy link
Collaborator

Hi @DevMrRober,
See below the changelog markdown before your change and after it.
Is this what you intended?
Or perhaps I am missing something?

BEFORE:

API Changelog 1.0.0 vs. 1.0.1

GET /api/{domain}/{project}/badges/security-score

  • ⚠️ removed the success response with the status '200'
  • ⚠️ removed the success response with the status '201'
  • ⚠️ deleted the 'cookie' request parameter 'test'
  • ⚠️ deleted the 'header' request parameter 'user'
  • ⚠️ deleted the 'query' request parameter 'filter'
  • api operation id 'GetSecurityScores' removed and replaced with 'GetSecurityScore'
  • api tag 'security' removed
  • for the 'query' request parameter 'token', the maxLength was increased from '29' to '30'
  • removed the pattern '^(?:[\w-./:]+)$' from the 'query' request parameter 'image'
  • for the 'query' request parameter 'image', the type/format was generalized from 'string'/'general string' to ''/''
  • removed the non-success response with the status '400'

GET /api/{domain}/{project}/install-command

  • ⚠️ deleted the 'header' request parameter 'network-policies'
  • added the new optional 'header' request parameter 'name' to all path's operations
  • added the new enum value 'test1' to the 'path' request parameter 'project'

POST /register

  • the endpoint scheme security 'bearerAuth' was removed from the API
  • the security scope 'write:pets' was added to the endpoint's security scheme 'OAuth'

AFTER:

API Changelog 1.0.0 vs. 1.0.1

GET /api/{domain}/{project}/badges/security-score

  • error: removed the success response with the status '200'

  • error: removed the success response with the status '201'

  • warning: deleted the 'cookie' request parameter 'test'

  • warning: deleted the 'header' request parameter 'user'

  • warning: deleted the 'query' request parameter 'filter'

  • api operation id 'GetSecurityScores' removed and replaced with 'GetSecurityScore'

  • api tag 'security' removed

  • for the 'query' request parameter 'token', the maxLength was increased from '29' to '30'

  • removed the pattern '^(?:[\w-./:]+)$' from the 'query' request parameter 'image'

  • for the 'query' request parameter 'image', the type/format was generalized from 'string'/'general string' to ''/''

  • removed the non-success response with the status '400'

GET /api/{domain}/{project}/install-command

  • warning: deleted the 'header' request parameter 'network-policies'

  • added the new optional 'header' request parameter 'name' to all path's operations

  • added the new enum value 'test1' to the 'path' request parameter 'project'

POST /register

  • the endpoint scheme security 'bearerAuth' was removed from the API

  • the security scope 'write:pets' was added to the endpoint's security scheme 'OAuth'

@DevMrRober
Copy link
Author

Hello @reuvenharrison

Yes, that's correct. My goal here is to highlight the severity of the breaking change, making it more descriptive and informative for users. This approach ensures that when a breaking change is introduced, users can immediately understand its impact. Additionally, this mechanism supports custom severity levels for greater flexibility

@reuvenharrison
Copy link
Collaborator

Okay, I understand that you added the "error:" and "warning:" titles.
Please note a couple of issues I'd like to address before merging this:

  1. Your solution introduced spaces between the lines - I'd rather avoid this unless there is a special reason for it
  2. Your solution omits the ⚠️ icon which was nice IMO

If the sole purpose of your PR it to differentiate between error and warning then perhaps we can use:
❗ or ⛔ for error
and
⚠️ for warning

See: https://github.com/ikatyang/emoji-cheat-sheet?tab=readme-ov-file#warning

@DevMrRober
Copy link
Author

How do you see this output?, for me it brings out the severity apart from the icon.

If you like it I add the changes to the PR

API Changelog 5.0.5 vs. 5.0.20

GET /users

  • ERROR the security scope 'read' was added to the endpoint's security scheme 'OAuth2'
  • ERROR the security scope 'write' was removed from the endpoint's security scheme 'OAuth2'

POST /users

  • WARNING removed the optional property 'data/description' from the response with the '201' status

PATCH /users/{userId}

  • WARNING removed the optional property 'data/description' from the response with the '200' status

PUT /users/{userId}

  • WARNING removed the optional property 'data/description' from the response with the '200' status

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.

2 participants