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

feat: setup nightly flow #82

Merged
merged 34 commits into from
Nov 17, 2023
Merged

feat: setup nightly flow #82

merged 34 commits into from
Nov 17, 2023

Conversation

melMass
Copy link
Collaborator

@melMass melMass commented Nov 9, 2023

Text for the squash merge:


reworks the CI logic a bit and introduces the idea of the nightly branch.

execution of the CI

  • ci.yml will trigger as usual
    • it won't run when changes are made to *.md files only
    • it will trigger on all PRs and pushes to main and nightly
    • it will run nupm-tests.yml
      • install Nupm with a new custom actions/setup_nupm
        main will use a pinned version (0.87 for now), nightly.... nightly.
      • run the tests of the module
  • in parallel, check_nightly.yml will make sure that PRs on main can be safely merged into nightly
  • finally, only when a PR on main is closed, the new main branch will be merged into nightly

TODO

  • a dedicated action for setup_nupm, could be used in other jobs later.
  • on pushes in PRs targeting main a new step is checked mergeable in nightly
  • on merge of PRs targeting main, main is automatically merged to nightly.

@melMass melMass changed the base branch from main to nightly November 9, 2023 21:03
@melMass melMass changed the base branch from nightly to main November 9, 2023 21:03
@melMass
Copy link
Collaborator Author

melMass commented Nov 9, 2023

Mmm for some reason it doesn't run the CI 🤣

@melMass melMass marked this pull request as ready for review November 9, 2023 21:10
@melMass melMass marked this pull request as draft November 9, 2023 22:41
This still needs further work and testing but the idea is there.
A few ideas I want to finish:
- use an explicit nushell version for the release tests.
  for now it uses "*" which is latest release.
- thanks to the previous point we can cache installs.
- for this logic to be sane main should be locked and only accept PR
@melMass melMass marked this pull request as ready for review November 12, 2023 15:57
Copy link
Owner

@amtoine amtoine left a comment

Choose a reason for hiding this comment

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

first: review on the main ci.yml => it looks great 🥳

i'll just push minor refactors and i have a few small questions 😌

then i'll review the two others 😏

.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/nupm-tests.yml Outdated Show resolved Hide resolved
.github/actions/setup_nupm/action.yml Show resolved Hide resolved
.github/actions/setup_nupm/action.yml Outdated Show resolved Hide resolved
@amtoine amtoine force-pushed the ci/setup_nightly_flow branch 2 times, most recently from 54e4020 to 8baafd0 Compare November 13, 2023 18:08
@amtoine amtoine force-pushed the ci/setup_nightly_flow branch from 8baafd0 to 01b971d Compare November 13, 2023 18:19
Copy link
Owner

@amtoine amtoine left a comment

Choose a reason for hiding this comment

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

i also have a few questions on the two new main actions 😉

.github/workflows/check_nightly.yml Show resolved Hide resolved
.github/workflows/check_nightly.yml Outdated Show resolved Hide resolved
.github/actions/sync_action/action.yml Show resolved Hide resolved
.github/actions/sync_action/action.yml Show resolved Hide resolved
@amtoine
Copy link
Owner

amtoine commented Nov 13, 2023

Important
the few force-pushes above were solely to make my pure-refactoring changes fit into one commit 😉

let's see if it works, it's much less readable 😅
@melMass melMass force-pushed the ci/setup_nightly_flow branch from 0af2c8e to d1cc0f0 Compare November 13, 2023 20:08
@melMass
Copy link
Collaborator Author

melMass commented Nov 13, 2023

Important
I reverted "make the tests manually triggerable" since it will be once merged I believe (and it broke the CI)

Not sure any output is needed if we use an input now
@melMass
Copy link
Collaborator Author

melMass commented Nov 14, 2023

There is just one thing we need to do before merge btw! (Add write rights for now push will fail)

@amtoine
Copy link
Owner

amtoine commented Nov 14, 2023

There is just one thing we need to do before merge btw! (Add write rights for now push will fail)

okey, you wanna do that?

there is a single thread missing, i don't know the syntax nor where to search for documentation for it 👀

@melMass
Copy link
Collaborator Author

melMass commented Nov 14, 2023

It's just a matter of using the builtin GitHub token I think, I will give it a quick look tonight

@amtoine
Copy link
Owner

amtoine commented Nov 15, 2023

Note
i've left the commits above (from 71168be to b3b7ac8 included) to show that i couldn't get #82 (comment) to work 😢

@amtoine amtoine force-pushed the ci/setup_nightly_flow branch from 03afd00 to 4e752d3 Compare November 15, 2023 19:00
@amtoine amtoine force-pushed the ci/setup_nightly_flow branch from 4e752d3 to 2a33119 Compare November 15, 2023 19:02
@amtoine
Copy link
Owner

amtoine commented Nov 15, 2023

Note
same here, the last two commits are a failed attempt to apply actions/runner#409 (comment)

@amtoine amtoine added ci Something related to the Continuous Integration dev Related to the dev experience labels Nov 16, 2023
@melMass melMass requested a review from amtoine November 17, 2023 00:51
Copy link
Owner

@amtoine amtoine left a comment

Choose a reason for hiding this comment

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

thanks @melMass for working on this, really much appreciated 🙏

let's land this PR and see how it goes 😇 🤞

@amtoine amtoine merged commit 390d25d into main Nov 17, 2023
4 checks passed
@amtoine amtoine deleted the ci/setup_nightly_flow branch November 17, 2023 14:10
@amtoine amtoine mentioned this pull request Nov 17, 2023
amtoine added a commit that referenced this pull request Nov 17, 2023
for both the *dry* check and the real sync, the GitHub action needs to
be able to perform a merge.
for this Git needs to be setup, otherwise the runner cannot commit and
create the merge commit, which [crashed the
CI](https://github.com/amtoine/nu-git-manager/actions/runs/6905039750)
after #82.

this PR
- moves the Git setup before the *merge* step
- always sets up Git, regardless of the value of `do_push` and
`$env.ACT`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Something related to the Continuous Integration dev Related to the dev experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants