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

ci: from jenkins to github action #626

Merged
merged 1 commit into from
Jan 25, 2024
Merged

Conversation

pdesoyres-cc
Copy link
Collaborator

@pdesoyres-cc pdesoyres-cc commented Nov 6, 2023

Fixes #625

Fixes #627

Fixes #637

@pdesoyres-cc pdesoyres-cc requested a review from a team as a code owner November 6, 2023 19:41
@pdesoyres-cc pdesoyres-cc force-pushed the ci/from_jenkins_to_ga branch 6 times, most recently from d073f23 to e8bdb81 Compare November 6, 2023 20:21
@pdesoyres-cc pdesoyres-cc force-pushed the ci/from_jenkins_to_ga branch from e8bdb81 to 750f074 Compare November 6, 2023 20:29
@pdesoyres-cc pdesoyres-cc force-pushed the ci/from_jenkins_to_ga branch from 08efaf9 to e6f9544 Compare November 8, 2023 10:44
@pdesoyres-cc pdesoyres-cc force-pushed the ci/from_jenkins_to_ga branch from e6f9544 to 25a65a7 Compare November 8, 2023 10:55
@pdesoyres-cc pdesoyres-cc force-pushed the ci/from_jenkins_to_ga branch 3 times, most recently from d2284a0 to 0e679aa Compare November 8, 2023 11:32
@pdesoyres-cc pdesoyres-cc changed the title ci: add build github action workflow ci: from jenkins to github action Nov 23, 2023
@pdesoyres-cc pdesoyres-cc force-pushed the ci/from_jenkins_to_ga branch 8 times, most recently from 349d388 to a481d82 Compare December 4, 2023 14:31
@CleverCloud CleverCloud deleted a comment from github-actions bot Dec 4, 2023
@pdesoyres-cc pdesoyres-cc force-pushed the ci/from_jenkins_to_ga branch from a481d82 to dd8bc90 Compare December 4, 2023 14:40
@pdesoyres-cc pdesoyres-cc force-pushed the ci/from_jenkins_to_ga branch 6 times, most recently from 9936418 to 8b337a1 Compare December 4, 2023 17:38
Copy link

github-actions bot commented Dec 4, 2023

🔎 A preview has been automatically published:

  • 🐧 linux 981f3bb2b53b6b687a480c375ba7bbf826cdbd3196551c418d64b26cefbbb7bb
  • 🍏 macos c00c8d5b365647c23e6427bafbf1fc1fa0caf954fc4e7b3a7ff3bfed905ca72d
  • 🪟 win 04a62aa12d33374743ea04a8c5b073d695129ed9ea7d010ba8ecb46fc00a6354
    This preview will be deleted once this PR is closed.

@pdesoyres-cc pdesoyres-cc force-pushed the ci/from_jenkins_to_ga branch 5 times, most recently from ac6d4e2 to 734e37d Compare December 5, 2023 08:29
Copy link
Member

@hsablonniere hsablonniere left a comment

Choose a reason for hiding this comment

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

Hi @pdesoyres-cc,

😍 What you did with this PR is amazing!! Well done 👏

I wrote a few remarks. I have one real concern but I'm not sure if it's blocking. Let me explain.

To build the bundle or not...

I was expecting the build to only build (with pkg) but not generate the archives (tar.gz, zip) and not create the bundles (rpm, deb). Therefore, I was expecting the preview links to directly refer to the binaries and not the archives. I guess what you did is a bit cleaner and smaller.

I would be OK to keep building the archives and using them for the preview links but I'm not sure we need to build the bundles for every PR as we don't do anything with them. In a way what you did ensures a PR cannot be merged if the bundles build breaks somehow so maybe we should keep it...

The only problem I have is I failed to install fpm on my system and cannot run the build job locally.

WDYT? Should we keep it as is?

About Docker Hub

Your PR does the same thing as what we did for Docker: create a commit and push it. It used to trigger build and publish on Docker's infra but it's not the case anymore. We should update the task so it builds the docker image and publishes it to the hub.

Can you create a separated issue for this?

Commits

  • This needs to be rebased on latest master
  • Do we want to keep this full history?
    • We could fixup the refactor and chore commits but if it's too much work, don't bother it's already good as is

.github/workflows/build.yml Show resolved Hide resolved
.github/workflows/build.yml Show resolved Hide resolved
.github/workflows/preview.yml Outdated Show resolved Hide resolved
.github/workflows/preview.yml Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
@pdesoyres-cc
Copy link
Collaborator Author

pdesoyres-cc commented Jan 19, 2024

Bundle or not bundle during the build phase?

  • I agree that having to install fpm even though we don't need the result is pointless.
  • In the meantime, I also like the build job to verify as many things as it can.

I have 2 propositions:

  • add a parameter to the job-build.js to skip the bundling phase.
    • we could go further and add another parameter that skips the archive phase which you also don't need locally.
    • locally, you could just do something like node scripts/job-build.js --skip-bundle --skip-archive
  • another proposition is to remove the bundling phase from the job-build.js but add job-bundle.js which only bundles.
    • locally you'll then run node scripts/job-build.js and it doesn't bundles.
    • we need to add the bundling phase to the build GitHub action.
    • the advantage is that we can install the dependencies needed by the bundling phase after the build, which may make the workflow fail faster in case of build failure.

I'm ok with both propositions, just let me know if you prefer one or another.

.github/workflows/build.yml Show resolved Hide resolved
.github/workflows/preview.yml Show resolved Hide resolved
.github/workflows/preview.yml Show resolved Hide resolved
.github/workflows/preview.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
scripts/archive.js Show resolved Hide resolved
scripts/cellar-client.js Show resolved Hide resolved
@pdesoyres-cc
Copy link
Collaborator Author

  • I rebased on master
  • I plan to squash all commits. Is it ok for you? @hsablonniere

@pdesoyres-cc pdesoyres-cc force-pushed the ci/from_jenkins_to_ga branch from 734e37d to cf9322b Compare January 19, 2024 15:31
@hsablonniere
Copy link
Member

hsablonniere commented Jan 25, 2024

@pdesoyres-cc

  • OK to squash all commits
  • Let's go for option 2 with opt-in

* build workflow
* preview workflow
* release with release-please

Fixes #625, #627, #637
@pdesoyres-cc pdesoyres-cc force-pushed the ci/from_jenkins_to_ga branch from cf9322b to a9864b6 Compare January 25, 2024 15:33
@miton18 miton18 self-requested a review January 25, 2024 15:51
@hsablonniere hsablonniere merged commit 059cbd4 into master Jan 25, 2024
5 checks passed
@hsablonniere hsablonniere deleted the ci/from_jenkins_to_ga branch January 25, 2024 15:52
Copy link

🔎 The preview has been automatically deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants