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: define the develop and main branch to run the actions on commit #71

Merged
merged 7 commits into from
Apr 10, 2024

Conversation

clauBv23
Copy link
Contributor

@clauBv23 clauBv23 commented Apr 9, 2024

Description

Changes addressed on this PR:

  • Run actions over all PR relevant PR changes.
  • Run the actions over the commit changes made on main.

Task ID: OS-1134

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • I ran all tests with success and extended them if necessary.
  • I have updated the CHANGELOG.md file in the root folder.

banasa44
banasa44 previously approved these changes Apr 9, 2024
Rekard0
Rekard0 previously approved these changes Apr 9, 2024
Copy link
Contributor

@josemarinas josemarinas left a comment

Choose a reason for hiding this comment

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

Correct me if I'm wrong but this wont trigger on prs

@josemarinas
Copy link
Contributor

Correct me if I'm wrong but this wont trigger on prs

nevermind, I think we are ok

josemarinas
josemarinas previously approved these changes Apr 10, 2024
@josemarinas
Copy link
Contributor

just fyi @Rekard0 , this reverts the changes on https://github.com/aragon/osx-commons/pull/63/files

We deleted the restrictions so we tested that changes on one packages does not break the other ones, this addresses this by testing everything on develop and main, if there are breaking changes it will fail but it will do it after we merge the PR

@clauBv23 clauBv23 dismissed stale reviews from josemarinas, Rekard0, and banasa44 via 317bccf April 10, 2024 10:34
@clauBv23
Copy link
Contributor Author

clauBv23 commented Apr 10, 2024

just fyi @Rekard0 , this reverts the changes on https://github.com/aragon/osx-commons/pull/63/files

The changes on that PR were done due to actions being triggered on changes on commits, not on PRs (on push instead of on pull_request).
That was causing action not to run on certain commits and PR seemed correct when it wasn't, for example here.

This PR is not reverting the changes made on PR #63. In the current approach, the modifications made on the defined path are scoped to the entire PR, not a single commit as was being done.

We deleted the restrictions so we tested that changes on one packages does not break the other ones, this addresses this by testing everything on develop and main, if there are breaking changes it will fail but it will do it after we merge the PR

Regarding this part, this repo packages are not related directly but through npm package dependencies.

I mean, we have four packages: contracts, SDK, subgraph, and config, and their dependencies are defined in the respective package.json. So if contracts need SDK it will have to define the dependency in the json file, specify the required version, and check everything continues working on the contract package.

The only packages that are directly related are the subgraph and the contracts because the subgraph needs the contracts abi and that is already managed

@clauBv23 clauBv23 merged commit c128667 into develop Apr 10, 2024
11 checks passed
@clauBv23 clauBv23 deleted the OS-1134/fix-GH-actions-to-run-over-PR-changes branch April 10, 2024 11:57
@clauBv23 clauBv23 changed the title feat: define the develop and main branch to run the actions on commit… feat: define the develop and main branch to run the actions on commit Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants