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

Rotational cipher #290

Merged
merged 7 commits into from Aug 16, 2017

Conversation

Projects
None yet
3 participants
@creaaa
Copy link
Contributor

creaaa commented Aug 16, 2017

Dear repository master,

Hello, I'm Masa, an iOS developer.
I was impressed by this project and added(ported) a new exercise. It's "Rotational cipher".

Would you take a look and merge if there's no problem?
I'm happy to fix my code if I'm making a mistake.

Thanks,
Masa

@pacman-bot

This comment has been minimized.

Copy link

pacman-bot commented Aug 16, 2017

3 Messages
📖 exercises/dominoes/Sources/DominoesExample.swift#L149 - Prefer != nil over let _ =
📖 exercises/dominoes/Sources/DominoesExample.swift#L152 - Prefer != nil over let _ =
📖 exercises/poker/Sources/PokerExample.swift#L189 - Function should have complexity 10 or less: currently complexity equals 12

Generated by 🚫 Danger

@creaaa

This comment has been minimized.

Copy link
Contributor Author

creaaa commented Aug 16, 2017

Dear maintainers,

Hello, I was making a mistake. I was overlooking about swiftlint and Travis CI.
Now lint doesn't complain about rotational-cipher directory I added and passed Travis CI as you can see.
Again, could you take a look? I’m sorry to bother you.

Sincerely,
Masa

@masters3d

This comment has been minimized.

Copy link
Member

masters3d commented Aug 16, 2017

Hi Masa!

Thanks for the contribution. A couple of small changes otherwise looks great!

Thanks.

import PackageDescription

let package = Package(
name: "rotational-cipher"

This comment has been minimized.

@masters3d

masters3d Aug 16, 2017

Member

We use camel case for the package name here.

@@ -0,0 +1,6 @@
import XCTest
@testable import rotational_cipherTests

This comment has been minimized.

@masters3d

masters3d Aug 16, 2017

Member

Camel case for test me too. Same as package.

@@ -0,0 +1,23 @@
identifier_name:
min_length:

This comment has been minimized.

@masters3d

masters3d Aug 16, 2017

Member

Use the config file on the root. I'd rather her not have nested config files. You are welcome to suggest changes to that file.

@@ -542,6 +542,14 @@
]
},
{
"slug": "rotational-cipher",
"difficulty": 4,

This comment has been minimized.

@masters3d

masters3d Aug 16, 2017

Member

Looks good!

@masters3d

This comment has been minimized.

Copy link
Member

masters3d commented Aug 16, 2017

Sorry, I forgot to push my comments. :(

creaaa added some commits Aug 16, 2017

@creaaa

This comment has been minimized.

Copy link
Contributor Author

creaaa commented Aug 16, 2017

Dear Jose,

I have to say thank you for your courteous code review.
Then I re-fixed some code so that it met repository’s requirement.
As you can see, swiftlint complaints nothing and passes Travis CI.
Would you take a look and let me know if I have some problem with that?

Thanks in advance.

sincerely,
Masa

@masters3d masters3d merged commit 2223ee5 into exercism:master Aug 16, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@masters3d

This comment has been minimized.

Copy link
Member

masters3d commented Aug 16, 2017

Thank you!

@creaaa creaaa deleted the creaaa:rotational-cipher branch Aug 16, 2017

@creaaa

This comment has been minimized.

Copy link
Contributor Author

creaaa commented Aug 16, 2017

I really appreciated it, Jose! 😭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.