Skip to content

Conversation

@felicio
Copy link
Collaborator

@felicio felicio commented May 19, 2025

@changeset-bot
Copy link

changeset-bot bot commented May 19, 2025

🦋 Changeset detected

Latest commit: 71cfa2c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
wallet Patch
connector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented May 19, 2025

@felicio is attempting to deploy a commit to the Status Team on Vercel.

A member of the Team first needs to authorize it.

@felicio felicio self-assigned this May 19, 2025
@status-im-auto
Copy link
Member

status-im-auto commented May 19, 2025

Jenkins Builds

Click to see older builds (27)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ ea274b1 #1 2025-05-19 10:27:51 ~51 sec chrome 📦zip
✔️ f5f2fdb #1 2025-05-22 16:58:53 ~39 sec chrome 📦zip
✔️ e2558bc #2 2025-05-22 17:07:36 ~37 sec chrome 📦zip
✔️ e2558bc #1 2025-05-22 17:08:20 ~50 sec chrome 📦zip
✔️ 53eee2f #1 2025-05-22 17:11:17 ~40 sec chrome 📦zip
✔️ 53eee2f #4 2025-05-22 17:11:21 ~36 sec chrome 📦zip
✔️ e25cf1f #2 2025-05-22 17:12:32 ~37 sec chrome 📦zip
✔️ e25cf1f #5 2025-05-22 17:12:50 ~1 min chrome 📦zip
✔️ e25cf1f #1 2025-05-22 17:13:42 ~49 sec chrome 📦zip
✔️ 31cd330 #6 2025-05-22 17:14:45 ~36 sec chrome 📦zip
✔️ 31cd330 #2 2025-05-22 17:14:52 ~45 sec chrome 📦zip
✔️ 31cd330 #1 2025-05-22 17:16:56 ~37 sec chrome 📦zip
✔️ 3af18e8 #3 2025-05-22 17:20:46 ~36 sec connector 📦zip
✔️ 3af18e8 #3 2025-05-22 17:20:52 ~36 sec connector 📦zip
✔️ d5ce1fe #4 2025-05-22 17:25:10 ~36 sec connector 📦zip
✔️ d5ce1fe #4 2025-05-22 17:25:17 ~36 sec connector 📦zip
✔️ d5ce1fe #1 2025-05-22 17:26:52 ~37 sec connector 📦zip
✔️ d5ce1fe #1 2025-05-22 17:26:57 ~37 sec connector 📦zip
✔️ 7b5b6aa #3 2025-05-22 17:28:41 ~36 sec connector 📦zip
✔️ 7b5b6aa #1 2025-05-22 17:28:41 ~36 sec connector 📦zip
✔️ 7b5b6aa #1 2025-05-22 17:29:42 ~37 sec connector 📦zip
✔️ 2f63a96 #2 2025-05-22 17:32:12 ~37 sec connector 📦zip
✔️ 2f63a96 #2 2025-05-22 17:32:17 ~36 sec connector 📦zip
✔️ 0067b51 #2 2025-05-22 17:35:21 ~36 sec connector 📦zip
0067b51 #2 2025-05-22 17:35:45 ~1 min wallet 📄log
✔️ 5e1a89c #3 2025-05-22 17:39:36 ~36 sec connector 📦zip
5e1a89c #3 2025-05-22 17:40:05 ~1 min wallet 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 58dceae #4 2025-05-26 10:10:55 ~37 sec connector 📦zip
✔️ 58dceae #4 2025-05-26 10:11:21 ~1 min wallet 📦zip
✔️ 01b507d #5 2025-05-26 10:22:09 ~1 min wallet 📦zip

@felicio
Copy link
Collaborator Author

felicio commented May 19, 2025

@markoburcul could you please enable builds for apps/wallet here like in #590 for apps/connector? And possibly distinguishing them by adding a new column called "Name" (i.e. connector and wallet)?

@felicio felicio moved this from In Progress to Waiting in Web & User Interfaces May 19, 2025
@felicio
Copy link
Collaborator Author

felicio commented May 19, 2025

@markoburcul you should be able to push to this branch should you need to add any changes, otherwise please let me know.

@markoburcul
Copy link
Contributor

@felicio I am not able to find the branch after cloning the repo:

status-web(main)» git checkout felicio:wallet-build                                                                                                                    [15:48:02]
error: pathspec 'felicio:wallet-build' did not match any file(s) known to git

@felicio
Copy link
Collaborator Author

felicio commented May 19, 2025

@markoburcul it's a fork  – https://github.com/felicio/status-web/tree/wallet-build. Please, lemme know if you cannot work with those for some reason.

@markoburcul
Copy link
Contributor

@markoburcul it's a fork – https://github.com/felicio/status-web/tree/wallet-build. Please, lemme know if you cannot work with those for some reason.

Why are you using forks? Why wouldn't we enable build for apps/wallet in the original repo?

@felicio
Copy link
Collaborator Author

felicio commented May 19, 2025

@markoburcul not everybody contributing will have a direct access nor you might want or need to necessarily share your branches upstream.

This is the upstream/original repo, and we'd like to enable them.

Even this PR, already built app/connector, check out 👇

image

#662 (comment)

@jakubgs
Copy link
Member

jakubgs commented May 20, 2025

We will not run PR jobs from external contributor forks in our CI. It is a major attack vector.

Also, you are a core contributor, you can push branches directly to this repo, why are you working with a fork?

@felicio
Copy link
Collaborator Author

felicio commented May 20, 2025

@jakubgs so this #662 (comment) ran because I'm part of the org and will continue to be built?

@jakubgs
Copy link
Member

jakubgs commented May 21, 2025

Yes, correct, the policy is that PRs from external contributors - meaning not in the org or added to the repo - need approval by contributors in order to allow PR CI jobs to run.

@jakubgs
Copy link
Member

jakubgs commented May 21, 2025

This can be refactored to use just one Jenkinsfile for both components. I can do that, but since this is using a branch from a fork I can't contribute to it currently. Why not just push a branch directly to this repo?

@felicio
Copy link
Collaborator Author

felicio commented May 21, 2025

Talk summary

  • issue created https://github.com/status-im/status-website/issues/1563
  • contributing directly to the branch is allowed and relevant git remote can be added locally
  • by following the forking flow for external contributors, we're ensuring it's well set-up and will work for them as expected too. also, anybody can be messy with branches and experiments in their respective forks as they want to
  • forks will be revisited but focusing on the builds now

@jakubgs jakubgs force-pushed the wallet-build branch 9 times, most recently from d5ce1fe to 7b5b6aa Compare May 22, 2025 17:27
@jakubgs
Copy link
Member

jakubgs commented May 22, 2025

There you go, I pushed a commit to this PR that refactors the Jenkinsfile to be usable for any app in the apps folder:

For now I made the choices only wallet and connector, but we can easily expand that list.

One problem that exists tho is that:

  • connector builds into build/chrome-mv3-prod
  • wallet builds into .output/chrome-mv3

Could we not consolidate those paths to be the same?

@felicio
Copy link
Collaborator Author

felicio commented May 26, 2025

java.io.IOException: /home/jenkins/workspace/atus-web_prs_linux_wallet_PR-662/apps/wallet/build/chrome-mv3-prod does not exist.

https://ci.status.im/job/status-web/job/prs/job/linux/job/wallet/job/PR-662/3/console

@felicio
Copy link
Collaborator Author

felicio commented May 26, 2025

Thanks @jakubgs.

One problem that exists tho is that:

For now pushed 955cbbe. Could you please approve the PR if agreed?

Could we not consolidate those paths to be the same?

Builds in /apps are handled by respective frameworks and not all expose a granular config per root output dir, target and environment. And adding a custom post build script to handle that now is not worth, I think.

@felicio felicio requested a review from jakubgs May 26, 2025 10:38
@felicio felicio assigned jakubgs and unassigned markoburcul May 26, 2025
@felicio felicio requested review from a team, jkbktl and marcelines May 26, 2025 13:58
@felicio felicio mentioned this pull request May 26, 2025
Copy link
Member

@jakubgs jakubgs left a comment

Choose a reason for hiding this comment

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

The Jenkinsfile looks good to me, though not sure what the other changes are about.

@felicio felicio merged commit 4500418 into status-im:main May 27, 2025
3 of 6 checks passed
@github-project-automation github-project-automation bot moved this from 👁️ Waiting to Done in Web & User Interfaces May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants