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 github action to sync shared source from aspnetcore #61999

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

Tratcher
Copy link
Member

@Tratcher Tratcher commented Nov 24, 2021

This mirrors an action we have in aspnetcore to sync changes to shared HTTP source code files.
https://github.com/dotnet/aspnetcore/actions/workflows/runtime-sync.yml

The one in aspnet triggers on a nightly schedule and sends a PR to aspnetcore if it detects any diffs. However, sometimes changes need to be submitted to runtime instead. The new action added here will only trigger manually, and will send a pr to runtime with any diffs.

@ghost
Copy link

ghost commented Nov 24, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

This mirrors an action we have in aspnetcore to sync changes from shared HTTP source code files.
https://github.com/dotnet/aspnetcore/actions/workflows/runtime-sync.yml

The one in aspnet triggers on a nightly schedule and sends a PR to aspnetcore if it detects any diffs. However, sometimes changes need to be submitted to runtime instead. The new action added here will only trigger manually, and will send a pr to runtime with any diffs.

Author: Tratcher
Assignees: Tratcher
Labels:

area-System.Net.Http

Milestone: -

@JamesNK
Copy link
Member

JamesNK commented Nov 24, 2021

A couple of questions:

  • How is the sync triggered?
  • How are reaction changes on the PR added? For example, a sync change renames a method and runtime code needs to react to that.

@Tratcher
Copy link
Member Author

A couple of questions:

  • How is the sync triggered?

It's triggered manually from the https://github.com/dotnet/runtime/actions page. Compare to this one that has a "Run Workflow" option: https://github.com/dotnet/runtime/actions/workflows/create-codespaces-prebuild.yml

  • How are reaction changes on the PR added? For example, a sync change renames a method and runtime code needs to react to that.

You'd check out the branch and push additional changes.

Copy link
Member

@JamesNK JamesNK left a comment

Choose a reason for hiding this comment

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

I don't know anything about github actions.

This approve is moral support for the feature 😄

@Tratcher
Copy link
Member Author

FYI, here is an example of the issue and PR the runtime->aspnetcore version of this script generates:
dotnet/aspnetcore#18943 (comment)
dotnet/aspnetcore#38614

This is also a case where the changes need to go into runtime instead. The new aspnetcore->runtime script only generates a PR in runtime, it references the same issue in aspnetcore.

@@ -0,0 +1,3 @@
# Check the code is in sync
$changed = (select-string "nothing to commit" artifacts\status.txt).count -eq 0
Copy link
Member

Choose a reason for hiding this comment

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

Is this extra file needed? We could just inline this command in the gh action?

Also, I wonder if using git diff would be easier? Something like

git diff -M -C -b --ignore-cr-at-eol --ignore-space-at-eol --exit-code > ..\artifacts\diff.txt
$changed = !$?

The only issue I see with git diff are untracked files, so we would need to run git add -N . before running the command. Just an idea to consider.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess the aspnetcore version of this file did a lot more to open and comment on the github issue. Inlining just this part might be easier now.
https://github.com/dotnet/aspnetcore/blob/main/.github/workflows/ReportDiff.ps1

Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

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

Love the idea, thanks for submitting this!

@Tratcher
Copy link
Member Author

I'm going to merge this so we can do a live test, and then iterate on feedback.

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

Successfully merging this pull request may close these issues.

5 participants