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

[WIP] tvOS complete support #383

Open
wants to merge 126 commits into
base: develop
Choose a base branch
from
Open

[WIP] tvOS complete support #383

wants to merge 126 commits into from

Conversation

elsassph
Copy link
Contributor

@elsassph elsassph commented Apr 9, 2018

  • launch in simulator
  • archive
  • package

@renep
Copy link
Contributor

renep commented Apr 10, 2018

I'm always very glad when someone adds a feature to the plugin and send me a pull request. But I'm a big fan of unit tests. If I had found test that verifies the change, I would have just have pressed the merge pull request button. So if you can add some tests that would be great. Otherwise I have to look into this myself.
You find plenty of unit test in the project. I use the spockframework now for tests and the test classes have the suffix Specification. The classes with the suffix Test are normal unit tests.
Adding tests for your change should not be that hard with spock, but if you prefer normal junit tests it's fine with me. 😉

@elsassph
Copy link
Contributor Author

It's still very early investigation (having some weird issues now); we have to validate it and add tests for sure.

@elsassph elsassph force-pushed the feature/tvos_archive_release branch from ee3fe8a to 1ab4883 Compare April 10, 2018 14:48
@elsassph
Copy link
Contributor Author

Added a couple tests, and I'll ask for help to add more, but so far so good: running on simulator, build/archive/package with inhouse certificate :)

TomasHubelbauer
TomasHubelbauer reviewed Apr 11, 2018
View reviewed changes
libxcode/src/test/groovy/org/openbakery/simulators/SimulatorControlSpecification.groovy Outdated
@@ -648,7 +647,7 @@ class SimulatorControlSpecification extends Specification {
}

@Unroll
def "test the runTime with value #runtimeType with name : #runtimeName and #deviceCount devices"() {
def "Ensure than the compilation runtime: #runtimeType with name : #runtimeName and #deviceCount devices is properly defined"() {
Copy link

Choose a reason for hiding this comment

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

Typo: that

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed ;)

@phatblat
Copy link
Contributor

😲 wow, this is a big PR

@renep
Copy link
Contributor

renep commented May 30, 2018

Yes it is a big pull request.
I also think it would make sense to make a web meeting before (or during) the merge of this pull request, so that you can enlighten me of the changes.

@renep
Copy link
Contributor

renep commented Dec 4, 2019

Hmm... I don't know what to do with this pull request. @shoebox are you still interested in merging this pull request? If so than we should get in touch so that you can enlighten me about the changes.

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

@TomasHubelbauer TomasHubelbauer TomasHubelbauer left review comments

@shoebox shoebox shoebox left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@elsassph @renep @shoebox @phatblat @TomasHubelbauer @deltatre-johann