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

Fix bug in configure_apply action #612

Merged
merged 2 commits into from
Nov 14, 2024
Merged

Fix bug in configure_apply action #612

merged 2 commits into from
Nov 14, 2024

Conversation

AliSoftware
Copy link
Contributor

@AliSoftware AliSoftware commented Nov 14, 2024

What does it do?

When a new encryption key is recently added and pushed to ~/.mobile-secrets by one developer, other developers will likely not yet have their own ~/.mobile-secrets repo pulled to that latest commit that includes the new encryption key.

This means that in those scenarios, after a new project is set up for configure and a new encryption key is added, any other developer who would be told to "please now run bundle exec fastlane run configure_apply" will have the command fail, because on their local machine their ~/.mobile-secrets would not yet have the new encryption key, and the preflight test we did about this at the start of the action would error early.

The fix is easy: make sure we only check for the presence of the encryption key after we have entered the prepare_repository block and thus pulled and checked out the correct commit of ~/.mobile-secrets first.

Related conversation: p1731606985135599/1731586195.925939-slack-C04PWEZSYFL

Checklist before requesting a review

  • Run bundle exec rubocop to test for code style violations and recommendations
  • Add Unit Tests (aka specs/*_spec.rb) if applicable
  • Run bundle exec rspec to run the whole test suite and ensure all your tests pass
  • Make sure you added an entry in the CHANGELOG.md file to describe your changes under the appropriate existing ### subsection of the existing ## Trunk section.
  • If applicable, add an entry in the MIGRATION.md file to describe how the changes will affect the migration from the previous major version and what the clients will need to change and consider.

When a new encryption key is recently added and pushed to `~/.mobile-secrets` by one developer, other developers will likely not yet have their own `~/.mobile-secrets` repo pulled to that latest commit that includes the new encryption key.

This means that in those scenarios, after a new project is set up for `configure` and a new encryption key is added, any other developer who would be told to "please now run `bundle exec fastlane run configure_apply`" will have the command fail, because on their local machine their `~/.mobile-secrets` would not yet have the new encryption key, and the preflight test we did about this at the start of the action would error early.

The fix is easy: make sure we only check for the presence of the encryption key _after_ we have entered the `prepare_repository` block and thus pulled and checked out the correct commit of `~/.mobile-secrets` first.
@AliSoftware AliSoftware added the bug Something isn't working label Nov 14, 2024
@AliSoftware AliSoftware requested review from wzieba and a team November 14, 2024 18:53
@AliSoftware AliSoftware self-assigned this Nov 14, 2024
Copy link
Contributor

@mokagio mokagio left a comment

Choose a reason for hiding this comment

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

Looks good.

I'd love to have tests for this but it seems like an integration test with various moving parts that would go beyond the scope of this fix PR.

@@ -24,21 +24,21 @@ _None_

### Bug Fixes

- Fix issue with post-processing of PR urls in the body of GitHub releases created via `create_github_release` [#610]
- Fix issue with post-processing of PR urls in the body of GitHub releases created via `create_github_release`. [#610]
Copy link
Contributor

Choose a reason for hiding this comment

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

💅👍

@AliSoftware AliSoftware merged commit 21d0336 into trunk Nov 14, 2024
6 of 10 checks passed
@AliSoftware AliSoftware deleted the fix/configure-apply branch November 14, 2024 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants