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

External tests maintenance #13810

Merged
merged 2 commits into from Dec 16, 2022
Merged

External tests maintenance #13810

merged 2 commits into from Dec 16, 2022

Conversation

nikola-matic
Copy link
Collaborator

@nikola-matic nikola-matic commented Dec 16, 2022

cameel
cameel previously approved these changes Dec 16, 2022
test/externalTests/ens.sh Outdated Show resolved Hide resolved
@cameel
Copy link
Member

cameel commented Dec 16, 2022

By the way, this is a situation where separate commits would be perfectly appropriate. Each one of the points in the description is pretty much an independent change.

@nikola-matic
Copy link
Collaborator Author

By the way, this is a situation where separate commits would be perfectly appropriate. Each one of the points in the description is pretty much an independent change.

Separate commits or PRs? I theory I agree, but having 3 commits seems like overkill to me, and we're gonna squash them in the anyway, no?

@nikola-matic nikola-matic merged commit 43cc4d0 into develop Dec 16, 2022
@nikola-matic nikola-matic deleted the external-tests-maintenance branch December 16, 2022 15:21
@cameel
Copy link
Member

cameel commented Dec 16, 2022

Separate commits or PRs? I theory I agree, but having 3 commits seems like overkill to me, and we're gonna squash them in the anyway, no?

Commits. And no, we don't have to squash them. We only really squash review fixes into the original commits. Or we squash everything when the commits are so messy that they make the history harder to understand than just having a single commit. But overall all that we require is clean history, going straight to the point without any of the dead ends you went through when working on a solution, that is easy to understand when you look at it in git. Having each logical change in a separate, atomic commit with a clear message is fine and actually recommended.

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.

None yet

2 participants