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

ensure-alphabetize should add a commit with alphabetized data if needed #991

Open
mlinksva opened this issue Jan 13, 2023 · 7 comments
Open

Comments

@mlinksva
Copy link
Contributor

That would be nice.

It's easy to cause the test to fail if one forgets how - sorts in a word or even adds superfluous space characters.

@mlinksva mlinksva mentioned this issue Jan 14, 2023
4 tasks
@WebSnke
Copy link
Contributor

WebSnke commented Jan 14, 2023

@mlinksva What exactly would resolve this problem? Changing the entry itself would break the link, right?

@WebSnke
Copy link
Contributor

WebSnke commented Jan 14, 2023

I'm going to leave open PRs that are failing the alphabetization test (which also can fail due to extra spaces, like this one) as motivation to fix #991 but feel free to make pass manually and I'll merge when you do.

The tests seem to fail when new countries are added...

@mlinksva
Copy link
Contributor Author

The tests sometimes fail when new countries are added because extra spaces are present, probably when adding a new line the via the web editor which indents to the previous level.

- name: Ensure Orgs are sorted
run: |
bundle exec script/alphabetize
git diff --color --exit-code

The above sorts and overwrites the file, as a side effect without any extra spaces. It fails if diff has any output, whether from extra spaces or need to sort.

The idea would be to commit the overwritten file if there is any diff, and push that commit, autocorrecting a file that differs from the canonical (including sorted) output rather than failing and requiring manual fixup.

@mlinksva
Copy link
Contributor Author

Hopefully just like this example https://github.com/marketplace/actions/add-commit#automated-linting

@mlinksva
Copy link
Contributor Author

Nearly working in #1011 (comment)

@WebSnke
Copy link
Contributor

WebSnke commented Jan 23, 2023

The tests pass now in #1043.

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

No branches or pull requests

2 participants