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 patching script for CoC modifications #910

Closed
tobie opened this issue Jul 5, 2022 · 5 comments · Fixed by #972
Closed

Add patching script for CoC modifications #910

tobie opened this issue Jul 5, 2022 · 5 comments · Fixed by #972

Comments

@tobie
Copy link
Contributor

tobie commented Jul 5, 2022

As discussed in #909, and requested in #851, we'd like to add a patch file for the CoC and a script to automate patching updates. Both should be versioned.

Pointers on best practices to do that are welcomed.

@ljharb
Copy link
Member

ljharb commented Jul 5, 2022

Absent an exemplar to follow, I envision a custom script that works in this way:

  1. the repo stores a SHA of the upstream CoC; a local patch file, and a local build output CoC file.
  2. the script fetches the contents of the SHA, applies the patch file, and commits the result
  3. we add a github action that fails if the script runs and produces a git diff

@Relequestual
Copy link
Contributor

Relequestual commented Nov 7, 2022

It looks like this issue has been on the agenda multiple times.
Could we get some expansion on what's required here? I'm not clear, even with the context of it blocking #851.

The Contributor Covenant CoC is updated VERY rarely. In the event it's updated, such as a major version, it doesn't feel likely that the patch would work. If there's a structural change, the diff isn't going to be applied well.

If the answer is, "OK, then we need a new patch at that point in time," I think that's OK, but it feels like overhead.

@ljharb
Copy link
Member

ljharb commented Nov 7, 2022

Every time it's come up, the sense has seemed to be that we need #910 (comment) next.

@tobie
Copy link
Contributor Author

tobie commented Nov 8, 2022

@ljharb, I think Ben has a point: if we're blocking on a technical implementation that's currently for a one-line diff, maybe we should reconsider building tooling for this, and instead do it manually, at least for now.

@ljharb
Copy link
Member

ljharb commented Nov 8, 2022

@tobie oh sure, i wasn't disagreeing with the point, i was just reiterating past consensus.

@Relequestual Relequestual linked a pull request Mar 10, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants