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(converter): add infoSummaryRefractorPlugin #3848

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

kowalczyk-krzysztof
Copy link
Contributor

@kowalczyk-krzysztof kowalczyk-krzysztof commented Feb 22, 2024

This plugin removes the summary fixed field of Info Object.

Refs SWG-9954

Copy link
Member

@char0n char0n left a comment

Choose a reason for hiding this comment

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

Looks good! Two small nitpicks detected.

@char0n char0n changed the title feat(apidom-converter): info summary plugin feat(apidom): add infoSummaryRefractorPlugin Feb 22, 2024
@char0n char0n changed the title feat(apidom): add infoSummaryRefractorPlugin feat(converter): add infoSummaryRefractorPlugin Feb 22, 2024
@char0n
Copy link
Member

char0n commented Feb 22, 2024

@kowalczyk-krzysztof notice I've change the commit msg and it's body.

Would be ideal if this PR will eventually gets sqashed+merged to following commit msg:

feat(converter): add infoSummaryRefractorPlugin

This plugin removes the summary fixed field of Info Object.

Refs #3697

The commit message header is consistent with the previous ones - feat(converter): add infoSummaryRefractorPlugin

The commit body explains what this plugin is actually about.

The commit footer references the public GitHub epic associated with converter: #3697

@kowalczyk-krzysztof
Copy link
Contributor Author

@kowalczyk-krzysztof notice I've change the commit msg and it's body.

Would be ideal if this PR will eventually gets sqashed+merged to following commit msg:

feat(converter): add infoSummaryRefractorPlugin

This plugin removes the summary fixed field of Info Object.

Refs #3697

The commit message header is consistent with the previous ones - feat(converter): add infoSummaryRefractorPlugin

The commit body explains what this plugin is actually about.

The commit footer references the public GitHub epic associated with converter: #3697

Thanks for the comment. I'll make sure to use this convention from now on.

This plugin removes the summary fixed field of Info Object.

Refs #3697
@char0n
Copy link
Member

char0n commented Feb 22, 2024

The tests are failing. Probably other tests got affected by new plugin.

@kowalczyk-krzysztof
Copy link
Contributor Author

kowalczyk-krzysztof commented Feb 22, 2024

The tests are failing. Probably other tests got affected by new plugin.

It was the change from info summary in test context() to info-summary which caused the snapshot to become obsolete. I've pushed a fix.

@char0n
Copy link
Member

char0n commented Feb 22, 2024

@kowalczyk-krzysztof please don't rebase the PRs into single commits. By doing this we loose all the history of the PR and comments attached to specific commit changes.

GitHub has a squash and merge functionality directly available:

image

After clicking it allows you to modify entire commit message:
image

@char0n char0n merged commit 1f0a39c into main Feb 22, 2024
8 checks passed
@char0n char0n deleted the feat/info-summary-plugin branch February 22, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants