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.

Sign up for GitHub

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

Jump to bottom

feat: add support for different tenants #4

Merged
merged 3 commits into from
Jan 16, 2024
Merged

feat: add support for different tenants #4

merged 3 commits into from
Jan 16, 2024

Conversation

mieliespoor
Copy link
Contributor

This adds the option to specify the NowSecure tenant endpoint.

We don't use the default NowSecure tenant, so for use to be able to use the plugin, we must be able to point to different tenants

@CLAassistant
Copy link

CLAassistant commented Dec 13, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@mieliespoor
Copy link
Contributor Author

@onyxmueller can you please review this one?

onyxmueller
onyxmueller suggested changes Jan 2, 2024
View reviewed changes
Copy link
Contributor

@onyxmueller onyxmueller left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi @mieliespoor! Sorry for the delay during the holidays. Thanks for your submission! This looks great. There's only one item I'd like to change. And it appears rubocop is not happy with the syntax.

https://github.com/WarnerMedia/fastlane-plugin-nowsecure/actions/runs/7194853439/job/20107611944?pr=4

lib/fastlane/plugin/nowsecure/actions/nowsecure_action.rb Outdated Show resolved Hide resolved
@marais-vzc
chore: mark option as optional
35f448d 
Signed-off-by: Marais Van Zyl <[email protected]>
@onyxmueller
Copy link
Contributor

onyxmueller commented Jan 16, 2024 •
edited
Loading

Thanks for updating the API @mieliespoor!

I think you might have missed my comment about rubocop not being happy with the syntax. Some fixes are needed for that. Let me see if it's something I can help identify.

@onyxmueller onyxmueller requested review from onyxmueller and removed request for onyxmueller January 16, 2024 20:28
@mieliespoor @onyxmueller
chore: apply suggestion
2078af6 
Co-authored-by: Onyx Mueller <[email protected]>
onyxmueller
onyxmueller approved these changes Jan 16, 2024
View reviewed changes
Copy link
Contributor

@onyxmueller onyxmueller 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. Thanks!

@onyxmueller
Copy link
Contributor

FYI, I will merge this but will publish the gem after I update the README with the new option. I will try to do that tonight or tomorrow.

@onyxmueller onyxmueller merged commit e78bc2d into WarnerMedia:main Jan 16, 2024
2 checks passed
@onyxmueller
Copy link
Contributor

Gem published under version 0.2.0. 🚀
https://rubygems.org/gems/fastlane-plugin-nowsecure/versions/0.2.0

Would you mind validating it works as expected from your end?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@onyxmueller onyxmueller onyxmueller approved these changes

Assignees

@mieliespoor mieliespoor

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mieliespoor @CLAassistant @onyxmueller @marais-vzc