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

meta: add cap diff check #950

Merged
merged 8 commits into from
May 28, 2021
Merged

meta: add cap diff check #950

merged 8 commits into from
May 28, 2021

Conversation

leighmcculloch
Copy link
Member

What

Add a GitHub Action that check the diffs in CAPs are valid.

Why

On a few occasions diffs have been committed that are corrupt. This is usually discovered by someone consuming the diff, and not by the person publishing the diff. This consumes that other persons time, as they need to debug the diff. Diffs are fiddly and it's easy for copy paste errors to occur, or stray characters, or missing new lines at the end to result in a corrupt diff. Instructions introduced in 9b0d752 should help us generate good diffs, but this check will catch the corrupt diffs when the author is submitting them, before someone else is consuming them.

Some of the existing CAPs were easy to update to pass the check. Others not, some just don't seem to apply cleanly to any version as far as I could see, which is the type of situation this check will prevent. For those cases I added a parameter to ignore them for the moment until their authors can fix them.

This GitHub Action uses a tool that I wrote, mddiffcheck, which lives at https://github.com/stellar/mddiffcheck. It took more effort than I expected or planned to give to it to write the tool, but it is done now.

Close #893

@leighmcculloch leighmcculloch marked this pull request as ready for review May 25, 2021 16:18
@leighmcculloch leighmcculloch marked this pull request as draft May 25, 2021 16:21
@leighmcculloch leighmcculloch marked this pull request as ready for review May 25, 2021 16:49
core/cap-0035.md Outdated Show resolved Hide resolved
marcelosalloum
marcelosalloum previously approved these changes May 25, 2021
Copy link
Contributor

@marcelosalloum marcelosalloum left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@MonsieurNicolas MonsieurNicolas left a comment

Choose a reason for hiding this comment

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

Nice! A few questions and suggestions

.github/workflows/mddiffcheck.yml Show resolved Hide resolved
.github/workflows/mddiffcheck.yml Outdated Show resolved Hide resolved
core/cap-0035.md Outdated Show resolved Hide resolved
leighmcculloch added a commit that referenced this pull request May 28, 2021
Replace CRLF line endings with LF line endings.

This is the only CAP in the repo that has CRLF line endings. Consistency would make some things easier. Folks on Windows can configure their git to autocrlf as needed for their workflows without introducing CRs upstream.

This was discovered in #950.
@leighmcculloch leighmcculloch merged commit f237a59 into stellar:master May 28, 2021
@leighmcculloch leighmcculloch deleted the mddiffcheck branch May 28, 2021 22:04
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.

meta: diff checker for CAPs
3 participants