Skip to content

Support global.json's rollForward latest* variants#538

Open
js6pak wants to merge 4 commits into
actions:mainfrom
js6pak:feat/support-rollForward
Open

Support global.json's rollForward latest* variants#538
js6pak wants to merge 4 commits into
actions:mainfrom
js6pak:feat/support-rollForward

Conversation

@js6pak
Copy link
Copy Markdown

@js6pak js6pak commented Jul 20, 2024

Description:
This PR is a cleaned up version of #481, which fixes 2 issues:

  1. In the original npm run build wasn't rerun so dist is stale there
  2. The tests that were added never passed successfully

There are a few additional commits in this PR:

  1. babb40f which fixes the 6 minute delay on windows runners caused by clear-toolcache.ps1
  2. d9dae35 makes sure tests run even before the PR workflow is manually accepted, this can be dropped if needed

Related issue:
Fixes #448, closes #481

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

@js6pak js6pak requested a review from a team as a code owner July 20, 2024 22:55
@js6pak
Copy link
Copy Markdown
Author

js6pak commented Aug 6, 2024

@HarithaVattikuti

@js6pak js6pak force-pushed the feat/support-rollForward branch 2 times, most recently from d3880ca to 4456588 Compare May 1, 2025 18:30
@js6pak
Copy link
Copy Markdown
Author

js6pak commented May 1, 2025

@HarithaVattikuti bump

@priyagupta108
Copy link
Copy Markdown
Contributor

Hi @js6pak,

Thank you for this contribution, this fills an important gap in global.json support! Here are a few review points:

  1. Rebase onto latest main. This will ensure your changes are up-to-date with the current codebase.

  2. Revert the workflow trigger changes. The branch filter removals across the CI workflow files (.github/workflows/*.yml) appear unrelated to this feature. Please revert those so they remain as-is.

Once those are addressed, happy to take another look. Thanks!

@priyagupta108 priyagupta108 self-assigned this May 6, 2026
js6pak added 2 commits May 7, 2026 21:26
This further enhances time savings seen in actions#213, down from a few minutes to seconds.
This means you are going to be notified of CI issues quicker, especially if you have to wait for a confirmation before running the workflows on your PR.
@js6pak js6pak force-pushed the feat/support-rollForward branch 4 times, most recently from c9d65e1 to 2306c4f Compare May 7, 2026 20:26
js6pak added 2 commits May 7, 2026 23:32
The newer versions are net8.0/net462+ only, making older .NET sdks more annoying to test
@js6pak js6pak force-pushed the feat/support-rollForward branch from 2306c4f to b7a286c Compare May 7, 2026 21:33
@js6pak
Copy link
Copy Markdown
Author

js6pak commented May 7, 2026

@priyagupta108 I've rebased the PR, so you can take a look. Workflow trigger changes are needed for external contributors to test whether their changes pass CI, so I'd recommend keeping them, but if you want to drop it, I'd do it after making sure you don't want any other changes to the actual feature (so I can test that CI works after requested changes :v).

@js6pak
Copy link
Copy Markdown
Author

js6pak commented May 8, 2026

@priyagupta108 Note the Basic validation workflow is broken on the main branch: https://github.com/actions/setup-dotnet/actions/workflows/basic-validation.yml?query=branch%3Amain, so it's unrelated to this PR.

@priyagupta108
Copy link
Copy Markdown
Contributor

Hi @js6pak, thanks for the quick rebase!

A few follow-up items to address:

  1. __tests__/e2e-test-csproj changes: Migrating the entire test project from MSTest to Newtonsoft.Json just to support a single latestMinor test case feels like a significant change, especially since that test depends on .NET Core 3.1, which has been EOL since December 2022. Would you consider reverting the migration and dropping the latestMinor test instead?

  2. actions/checkout version: Some of the new jobs added in e2e-tests.yml still use actions/checkout@v3. Could these be updated to actions/checkout@v6 to match the rest of the workflow?

  3. Branch filters: The current branch filters (main, releases/*) are intentional because they prevent CI from running on every push to every branch, which helps keep workflow costs and noise low for the repo. Could you please drop this change from the PR once your testing is complete?

Once those are addressed, happy to continue the review. Thanks!

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.

Script crashes when rollForward feature is set to latestMinor

2 participants