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: enabling tests for PRs targeting v4 #739

Merged
merged 2 commits into from
Jan 23, 2023
Merged

Conversation

JeneaVranceanu
Copy link
Collaborator

Summary of Changes

PRs that target develop-4.0 branch will now run tests in CI.

Test Data or Screenshots

By submitting this pull request, you are confirming the following:
  • I have reviewed the Contribution Guidelines.
  • I have performed a self-review of my own code.
  • I have updated my repository to match the develop branch.
  • I have included test data or screenshots that prove my fix is effective or that my feature works.
  • I have checked that all tests work and swiftlint is not throwing any errors/warnings.

PRs that target develop-4.0 branch will now run tests in CI.
janndriessen
janndriessen previously approved these changes Jan 23, 2023
@@ -7,6 +7,8 @@ on:
- develop
- hotfix
- unstable
# Temporary develop-X.Y.Z branches may be added and removed from here as we release new versions
- develop-4.0
Copy link
Collaborator

@yaroslavyaroslav yaroslavyaroslav Jan 23, 2023

Choose a reason for hiding this comment

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

This given section was added in the past, where I'd simply pushes some changes directly at the branch without any review. While it could be useful to push some code directly sometimes, I assume that it is so only in case of urgency, like the prequel merge corrupt branch somehow and now it's needed to be fixed asap.

Long story short: I suggest to not add any new branches here, and consider to do something with this whole section in future, simultaneously hardening push rules to develop and master.

And why does it matter at all: action just duplicating actually (or at least was, didn't checked it quite a bit) on every PR and then on every commit merge. If I'm wrong please tell me about it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd remove this section completely. develop and master branches should be protected and direct pushes to these must be prohibited for everyone.

What would you say if I remove this block with push: rule? All CI Swift test runs must be complete before we merge anything.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is hard.
Actually as for now there's a several tasks that i'm doing locally and them pushing it to the master. One is merge devel to master, but this one i certainly can do through PR merge option by GitHub. The second one is updating podspec within master for every given release. Apparently i can do the same way as i'm doing it now, coz i having admin rights, but i don't like that. The perfect state would be if it was handled by GitHub actions as well, i mean creating release, or at least merging it. But for updating podspec i have doubts that this is possible, because of Cocoapods modularity restrictions: e.g. I have to push into pods Web3Core at first and then i could push there Web3Swift itself only after a day or so. So if you have any ideas how we can handle this edge case I'd be happy to hear.

Anyway my state is that cocoapods is an unloved child here, so if we have to impair it in anyway in sake of something valuable we should do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, then to make it easy I'll remove unstable and whatever else I've added to the push rule and we can discuss CocoaPod updates in a separate issue.

@yaroslavyaroslav yaroslavyaroslav merged commit 9a227f8 into develop Jan 23, 2023
@yaroslavyaroslav yaroslavyaroslav deleted the ci/test-v4-PRs branch January 23, 2023 18:47
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.

3 participants