Skip to content

iOS TOTP OSS 仕様準拠の確認#1

Closed
NMai-source wants to merge 11 commits into
ykws:mainfrom
NMai-source:main
Closed

iOS TOTP OSS 仕様準拠の確認#1
NMai-source wants to merge 11 commits into
ykws:mainfrom
NMai-source:main

Conversation

@NMai-source
Copy link
Copy Markdown
Collaborator

設定画面の追加

@ykws ykws added the good first issue Good for newcomers label Oct 14, 2021
@ykws ykws linked an issue Oct 14, 2021 that may be closed by this pull request
15 tasks
Comment thread Setting Controller.m
@@ -0,0 +1,8 @@
//
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

must-badge

不要ファイルだと思うので削除しておいてください

Comment thread Podfile.lock
PODFILE CHECKSUM: 07904d67eb13b9136ed0ec5bcece3229fe2f25a7

COCOAPODS: 1.10.2
COCOAPODS: 1.11.0
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

must-badge

ツールのバージョンのアップデートは別の PR として分けて欲しいです。

Copy link
Copy Markdown
Owner

@ykws ykws left a comment

Choose a reason for hiding this comment

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

must-badge

コミット 56dae2b の Author がそれまでと変わっていました。

  • 該当するコミット : Author: 中神舞 <nakagamimai@nakagamimainoMacBook-puro.local>
  • それまでのコミット: Author: Nakagami Mai <nm30.mmn0kg@gmail.com>

おそらく PC を変えた影響だと思うのですが、
git config で user.name, user.email を設定してください。

@ykws
Copy link
Copy Markdown
Owner

ykws commented Oct 15, 2021

@NMai-source バッジの意図についてはこちらを参考にして付けています
https://qiita.com/iganin/items/aee297eade84849cc9cd

ラベル 意味
must-badge 対応必須 この対応がされていないとマージを認められない
imo-badge in my opinion 自分ならこう実装するけど、どう?
ask-badge 実装意図の確認
nits-badge 細かい指摘(コードを整える、不要な改行など)
suggestion-badge 提案、このようにしたらどうか( imo との違いは客観的に推奨されている実装の提案)
good-badge 良い点
memo-badge 次の段階に進むためのポイント
memo-badge コード理解のためのメモとしてのコメント

@ykws
Copy link
Copy Markdown
Owner

ykws commented Oct 15, 2021

@NMai-source 作成が後手に回ってしまったのですが、以下の Pull Request Template に合わせて説明欄の変更をお願いします。

https://github.com/ykws/OneTimePasswordExample/blob/main/.github/pull_request_template.md

@ykws
Copy link
Copy Markdown
Owner

ykws commented Oct 15, 2021

@NMai-source #3 設定画面の追加までをこの Pull Request とできるように branch を分けてもらえますか?

アルゴリズムの選択などは #2 の Issue 対応として、この Pull Request とは別で作成してください。

@ykws
Copy link
Copy Markdown
Owner

ykws commented Oct 15, 2021

@NMai-source main に CI を導入したのでこの Pull Request に main を merge or rebase してみてください。

コミットがきれいになるので rebase がおすすめです。
以下、一例です。

手順の例

  1. リモートリポジトリに Pull Request 先のこのリポジトリを追加する(名前は何でも良いですが upstream が慣習です)
  2. リモートリポジトリの内容を取り込みます
  3. 今の branch に対してリモートの main の変更を取り込んで一本のコミットログに整形します
  4. 3の手順に変わって単に merge しても良いですが、マージコミットが発生してしまうのでコミットログが読みにくくなります

コマンドの例

git remote add upstream git@github.com:ykws/OneTimePasswordExample.git
git fetch upstream
git rebase upstream/main

@ykws ykws mentioned this pull request Oct 19, 2021
1 task
@ykws ykws added the duplicate This issue or pull request already exists label Oct 27, 2021
@ykws
Copy link
Copy Markdown
Owner

ykws commented Oct 27, 2021

#10 と重複するのでこちらは閉じます。

@ykws ykws closed this Oct 27, 2021
@ykws ykws mentioned this pull request Oct 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists good first issue Good for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants