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 improvements #605

Open
4 of 6 tasks
sakulstra opened this issue Sep 27, 2024 · 0 comments
Open
4 of 6 tasks

CI improvements #605

sakulstra opened this issue Sep 27, 2024 · 0 comments
Assignees
Labels
enhancement New feature or request

Comments

@sakulstra
Copy link
Collaborator

sakulstra commented Sep 27, 2024

Currently we got various jobs running in various workflows:

  • cron addresses update -> creates a pr with new onchain addresses
  • cron abi updates (noticed we actually don't have that) -> should create a pr with new abi updates
    • npm run generate:abis
  • build size check -> for the root package we want to track build size
    • the package we're using here seems outdated and throws some errors on ci
  • verification check -> similar to build size we want to track changes in address verification status
    • this job exists, but currently to read the results you need to check the actual action run manually
  • sanity checks -> defacto this is just a suite of javascript tests... not sure why i did it as a script. Probably should just be some unit test, but we don't have any js test suite in here yet.
    • currently this check just errors and results need to be checked manually
  • foundry test -> checks the project builds (while it's just constants there were issues in the past with special chars etc in the generated code)

edit: we also got dependabot and i think currently don't ensure that the ui still builds after a dependency update which has cause issues in the past.


Besides the problems mentioned above, one core issue we're facing here (and in other repos) is that for external contributions:
e.g. here: #603 cis are failing. The problem is that in this case for actually running tests like the verification check they need access to certain secrets, but they are not available for prs of outside contributors.

What people seem to do is:

  1. disable cis for external pull_requests and merge to a new local branch. This is extremely annoying as you need to manually create a branch, change the base of their pr, merge, create a new pr, ask for other ppl to review as you cannot "approve yourself"...
  2. use external cis and only trigger them from github (e.g. trigger travis from github). The downside here is that people can no longer easily see within github why their stuff fails - quite annoying as well.
  3. https://github.com/imjohnbo/ok-to-test/tree/master - things in the line of this. The idea is that a collaborator has to comment sth. In this case /ok-to-test sha=sth to trigger a ci run with elevated permissions. Probably not so high effort, might be less annoying then the other options. Didn't yet check in detail what opens there are.

Another issue we're facing is that because "js" is always treated as a second class citizen that we add "because it might help people" we currently have a wild mixture of js/bun/yarn/pnpm (even in this repo alone we apparently use yarn for address book and npm for ui 😅 ).
Our release job atm only works with yarn though, so for some packages we actually build them locally and manually push to npmjs.

I think would be reasonable to either:

  • align on one package manager (we're not doing rocket science - most packages we have, have close to zero dependencies... npm could just be fine)
  • make the release job somewhat agnostic

Tasks

@sakulstra sakulstra added the enhancement New feature or request label Sep 27, 2024
@boredland boredland self-assigned this Oct 29, 2024
@sakulstra sakulstra mentioned this issue Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants