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

fix(evasive-transform): replace homoglyphs with boring ascii #1841

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

boneskull
Copy link
Contributor

This eschews homoglyphs in the comment evasion transform because they are "too clever" and not "tamper-evident" enough. But I think it's just envy.

cc @kriskowal

@boneskull
Copy link
Contributor Author

@kriskowal Not in a hurry with this one, but curious if we are waiting for more eyes--or how exactly that works 'round these parts.

@kriskowal
Copy link
Member

In this case, you’ve got my stamp and I think I speak for the Agoric team that this is a preferable behavior. We generally prefer for the submitter to land if they’re a member of the org, so they have an opportunity to groom the history with a local rebase on origin/master before merging thru the Github UI.

We like to keep the git history looking like a single railway line with non-overlapping sidings for PRs, though we understand that’s not always possible. We like to have individually reviewable commits, but also understand there are many cases where a single commit will suffice. We still like to use the merge commit from the Github UI to preserve the signature of the individual commits, if there was a signature.

I’ve sent you an invitation to join the @endojs org which should make it possible for you to land this.

For Agoric SDK, this is our policy for commit history management https://github.com/Agoric/agoric-sdk/wiki/Commit-Management. However, we do not use a merge queue on Endo yet. We don’t have enough concurrent work to require it yet.

@boneskull boneskull force-pushed the boneskull/destroy-homoglyphs branch from 62189f0 to dc00caa Compare November 3, 2023 19:54
@kriskowal kriskowal merged commit 64a5c56 into endojs:master Nov 6, 2023
14 checks passed
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