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

Grammar & Clarity Fixes #15882

Closed
wants to merge 6 commits into from
Closed

Grammar & Clarity Fixes #15882

wants to merge 6 commits into from

Conversation

NeoByteXx
Copy link

File: test/externalTests/brink_test.sh
Before: "Remove this when Brink merges"
After: "Remove once Brink merges PR #52"
Fixed grammar in flaky test comment

File: test/externalTests/elementfi_test.sh
Before: "one particular cases" → "one particular case"
Before: "depends" → "depend"

Copy link

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

Copy link

@koagonzalo11 koagonzalo11 left a comment

Choose a reason for hiding this comment

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

good

@NeoByteXx
Copy link
Author

@clonker

@clonker
Copy link
Member

clonker commented Feb 21, 2025

Thanks! I see that you made a merge commit and also otherwise several commits which logically do essentially the same. We have the convention that we don't use merge commits as they can make the history hard to follow and also commits should contain one logical change. Which - in my opinion - fixing these typos is. A rebase onto origin/develop will get rid of the merge commit, squashing can be performed, e.g., by using interactive rebase on the last commits. Don't forget to force push afterwards to rewrite history on your branch :)

@NeoByteXx NeoByteXx closed this by deleting the head repository Feb 22, 2025
@NeoByteXx
Copy link
Author

#15885

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants