Navigation Menu

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

[SR-1187] Add example test to Dealer example #5304

Closed
rballard opened this issue Apr 7, 2016 · 8 comments
Closed

[SR-1187] Add example test to Dealer example #5304

rballard opened this issue Apr 7, 2016 · 8 comments
Assignees

Comments

@rballard
Copy link
Contributor

rballard commented Apr 7, 2016

Previous ID SR-1187
Radar None
Original Reporter @rballard
Type Improvement
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Package Manager
Labels Improvement, StarterBug
Assignee @abertelrud
Priority Medium

md5: acc58f7cfd300680265cc96d1f2b46a7

Issue Description:

Our example packages don't currently have any tests. We should have at least one test in the package tree rooted at Dealer; preferably several. Testing is an important feature and our examples should demonstrate this.

Note: the dealer package is https://github.com/apple/example-package-dealer

@masters3d
Copy link
Contributor

I added these PR:
apple/example-package-deckofplayingcards#3
apple/example-package-fisheryates#6
apple/example-package-playingcard#1

I did not add tests to https://github.com/apple/example-package-dealer
but the package still works after the PR have been applied to it.

I did not test on Linux but I did add the necessary code for it to work.

@rballard
Copy link
Contributor Author

rballard commented May 5, 2016

Thanks for doing this, and my apologies that I've left this so long.

Max, can you please review this and accept it if appropriate?

@rballard
Copy link
Contributor Author

Anders, please review this and pull it in with any needed tweaks if appropriate.

@abertelrud
Copy link
Contributor

Reviewing.

@abertelrud
Copy link
Contributor

The tests look good in general; thanks!

Just a couple of things:
a) In addition to adding the tests, the diffs also slightly change the method names in the examples (e.g. `shuffle`); I think this is fine, but wanted to call it out to make sure it was intentional, since this SR just mentions adding a test
b) Is the change to use lowercase enums to make it conform to the new naming convention? I haven't been following this in detail, but am guessing that's the new convention?
c) The PRs no longer merge cleanly, though, so they would need to be updated to the latest.

Thanks!

@abertelrud
Copy link
Contributor

Again, apologies that this has been sitting in limbo for a long time!

@masters3d
Copy link
Contributor

a) b) these are meant to match the swift API naming guidelines.
c) I created a new PR apple/example-package-fisheryates#6
I believe the others are still good.

@abertelrud
Copy link
Contributor

Great, thanks! I've merged these.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 4, 2022
This issue was closed.
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

No branches or pull requests

3 participants