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

Handle multiple apps #4

Closed
wants to merge 3 commits into from
Closed

Handle multiple apps #4

wants to merge 3 commits into from

Conversation

jetersen
Copy link
Contributor

@jetersen jetersen commented Oct 7, 2017

  • Fetch all apps if no identifier specified.
  • Handle multiple identifiers

fixes #3

Copy link
Contributor Author

@jetersen jetersen left a comment

Choose a reason for hiding this comment

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

Feels like we need a 'Are you sure' before actually removing testers? 🤔

image

UI.message("Fetching the #{app_identifiers.size} apps, you specified, give me a moment 🤖")
app_identifiers.each do |app_identifier|
app = Spaceship::Application.find(app_identifier)
unless app
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could this be more Ruby like or is it good enough?

@@ -45,12 +63,12 @@ def self.run(params)
end
end

def self.remove_tester(tester, app_id, dry_run)
def self.remove_tester(tester, app, dry_run)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hardly remember ids, so lets pass app object and show app name when printing and use apple_id when deleting.


all_testers = Spaceship::TestFlight::Tester.all(app_id: app_id)
counter = 0
Copy link
Contributor Author

@jetersen jetersen Oct 7, 2017

Choose a reason for hiding this comment

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

We went from mere 4 testers being removed on our most popular test app
To now 164 when rolling over all apps 👍
Only took 27 seconds to go through 🤖 . Yet for a human.... 😱

@jetersen
Copy link
Contributor Author

@KrauseFx when you have time 👍 It's been pretty solid for house cleaning.

@jetersen
Copy link
Contributor Author

@KrauseFx will the PR be open for merging after I fix conflict or should I not even bother 👎 We definitely been using this feature for our use case.

@KrauseFx
Copy link
Member

Hey @Casz, sorry for the late reply, thanks for your PR 👍 Quick question: as a workaround, can users just call the plugin multiple times for each app, right?

@jetersen
Copy link
Contributor Author

jetersen commented Jan 22, 2018

Yes, they could, but why not give them the power 👍

Here we gave them 3 options.
specify one and we clean the single.
specify multiple in comma separated and we clean the multiple.
specify none and retrieve all and we clean ALL 👍 (Can't remember half of our bundle ids, not without looking)

@jetersen
Copy link
Contributor Author

@KrauseFx I'll try and rebase and hope you will pick this up! Hate to see this go to waste

* Fetch all apps if no identifier specified.
* Handle multiple identifiers
@jetersen
Copy link
Contributor Author

@KrauseFx rebased
To answer your question again, yes you can just pass one at a time but not all of can remember every thing apple id that we maintain 👍

@jetersen
Copy link
Contributor Author

@KrauseFx travis is very different from circle CI now it seems 😢

@jetersen jetersen closed this Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support getting all testers instead of specifying an app_id
2 participants