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 a codespell action to keep it clean of future typos #1

Closed
wants to merge 5 commits into from

Conversation

peternewman
Copy link
Contributor

No description provided.

@ssilverman
Copy link
Owner

Sorry to have missed this PR. I didn't get an email notification for some reason. My preference is to do one PR per change. (I've already fixed the typo, thanks for finding that.)

@peternewman
Copy link
Contributor Author

Sorry to have missed this PR. I didn't get an email notification for some reason.

No worries.

My preference is to do one PR per change.

Okay, do you want me to cherry-pick that commit out?

(I've already fixed the typo, thanks for finding that.)

Perhaps you haven't pushed then, it's still there currently:
https://github.com/ssilverman/rdm-schema/blame/e61c61573f1b078dffb779d5c9b1a6aaa1947a48/CHANGELOG.md#L413

@ssilverman
Copy link
Owner

That’s correct, I haven’t pushed yet.

@peternewman peternewman changed the title Fix a typo and add a codespell action to keep it clean Add a codespell action to keep it clean of future typos Sep 24, 2020
@peternewman
Copy link
Contributor Author

This is now just a single change PR @ssilverman .

@peternewman
Copy link
Contributor Author

Bump @ssilverman

@ssilverman
Copy link
Owner

ssilverman commented Jul 28, 2021

Question: Will this action prevent a merge or just show some notification if there's a typo?

Also, could you squash this into a single commit? Only one file is changed, but there are 4 commits. My workflow is to do a rebase for linear history. (I know there's other approaches.)

@peternewman
Copy link
Contributor Author

Question: Will this action prevent a merge or just show some notification if there's a typo?

That depends how/whether you configure branch protection:
https://docs.github.com/en/github/administering-a-repository/defining-the-mergeability-of-pull-requests/about-protected-branches

If you do nothing, this will just add an annotation against any PRs, like so:
https://github.com/peternewman/rdm-schema/pull/7/files

As well as tick/cross status against commits:
https://github.com/peternewman/rdm-schema/pull/7/commits

But with branch protection you can, if you want, only allow either PRs (or pushes too) to be merged when there are no spelling errors, to keep the branch clean. I'd say this will be particularly important for the validation PRs #6 #8 and #9 so you don't end up with broken JSON being merged in future.

You can also add words to skip:
https://github.com/codespell-project/actions-codespell/#parameter-ignore_words_list

Or if necessary, files:
https://github.com/codespell-project/actions-codespell/#parameter-skip

Also, could you squash this into a single commit?

See the discussion elsewhere...

Only one file is changed, but there are 4 commits.

One added the action, one fixed the typo (which I think you'd already fixed but not pushed at the time) and the other two were resyncing to try and get that fix in).

@ssilverman
Copy link
Owner

I think I see why more commits are being added. When I refer to "combining commits", I'm thinking of "interactive rebasing" and literally combining commits, not merging them. For example, if there's a few commits that only result in one file, you can use interactive rebase with the "f" command:

# f, fixup <commit> = like "squash", but discard this commit's log message

@ssilverman
Copy link
Owner

@peternewman Would you like to hop on a screen sharing call and we can get all your changes in together? I'd really like to get your fixes and additions in.

@peternewman
Copy link
Contributor Author

I think I see why more commits are being added. When I refer to "combining commits", I'm thinking of "interactive rebasing" and literally combining commits, not merging them. For example, if there's a few commits that only result in one file, you can use interactive rebase with the "f" command:

# f, fixup <commit> = like "squash", but discard this commit's log message

See #6 (comment) I'm pretty certain that will keep you happy! I think this is the web version of fix-up.

@peternewman Would you like to hop on a screen sharing call and we can get all your changes in together? I'd really like to get your fixes and additions in.

Thanks for the offer, unfortunately now doesn't work too well for me, I'm not really free for something like that until in a weeks time.

@peternewman peternewman mentioned this pull request Aug 9, 2021
@peternewman
Copy link
Contributor Author

Hi @ssilverman ,

Is #10 acceptable for you to merge instead of this?

Otherwise I should be able to fit in a screen sharing call at some point over the next few weeks if you really feel we need one. Do you want to drop me an email with some dates that work for you (if you've still got my email)?

@peternewman
Copy link
Contributor Author

Merged as a GitHub squash via #10.

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