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

Add CONTRIBUTION.md #1202

Merged
merged 3 commits into from Dec 17, 2015
Merged

Add CONTRIBUTION.md #1202

merged 3 commits into from Dec 17, 2015

Conversation

@garaemon
Copy link
Member

garaemon commented Oct 24, 2015

No description provided.

@garaemon garaemon force-pushed the garaemon:contribution branch from f1fe1ef to fcf8a73 Oct 24, 2015
@mmurooka

This comment has been minimized.

Copy link
Member

mmurooka commented Oct 24, 2015

このコーディングスタイルになるような.emacsの設定がもしありましたら,
教えて頂きたいです.

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Oct 25, 2015

Code Styleに関してではなくて,Pull Requestに関してですが,

## bashrc
_what_ros_package () {
    looking_path=$(pwd) 
    found=$(find $looking_path -maxdepth 1 -iname package.xml | wc -l) 
    while [ $found -eq 0 ]
    do
        looking_path=$(dirname $looking_path) 
        [ "$looking_path" = "/" ] && return
        found=$(find $looking_path -maxdepth 1 -iname package.xml | wc -l) 
    done
    echo $(basename $looking_path)
}
_git_commit_verbose () {
    tmp_file=$(mktemp -t XXXXXX) 
    ros_package=$(_what_ros_package) 
    if [ "${ros_package}" != "" ]
    then
        echo "[${ros_package}] " > ${tmp_file}
        git commit --verbose --template ${tmp_file}
    else
        git commit --verbose
    fi
}
alias gc='_git_commit_verbose'

と追加すると,あるROSパッケージの中で$ gcとすると,↓のような感じで勝手に[${package}]を入れてくれます.
screen shot 2015-10-25 at 11 40 30 am

@k-okada

This comment has been minimized.

Copy link
Member

k-okada commented Oct 25, 2015

CHANGELOG.rst はパッケージ毎にできるので,実はもう一段回深いとうれしいです.
ros/rosdistro#8978
e5cf7e3#diff-d0ea6af8aa6fbf40462a5c00380c59ad

◉ Kei Okada

On Sun, Oct 25, 2015 at 11:42 AM, Kentaro Wada notifications@github.com
wrote:

_Code Style_に関してではなくて,_Pull Request_に関してですが,

bashrc

_what_ros_package () {
looking_path=$(pwd)
found=$(find $looking_path -maxdepth 1 -iname package.xml | wc -l)
while [ $found -eq 0 ]
do
looking_path=$(dirname $looking_path)
[ "$looking_path" = "/" ] && return
found=$(find $looking_path -maxdepth 1 -iname package.xml | wc -l)
done
echo $(basename $looking_path)
}
_git_commit_verbose () {
tmp_file=$(mktemp -t XXXXXX)
ros_package=$(_what_ros_package)
if [ "${ros_package}" != "" ]
then
echo "[${ros_package}] " > ${tmp_file}
git commit --verbose --template ${tmp_file}
else
git commit --verbose
fi
}
alias gc='_git_commit_verbose'

と追加すると,あるROSパッケージの中で$ gcとすると,↓のような感じで勝手に[${package}]を入れてくれます.
[image: screen shot 2015-10-25 at 11 40 30 am]
https://cloud.githubusercontent.com/assets/4310419/10713782/3f507284-7b0d-11e5-984a-5c204c7eeaca.png


Reply to this email directly or view it on GitHub
#1202 (comment)
.

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Oct 25, 2015

@k-okada

CHANGELOG.rst はパッケージ毎にできるので,実はもう一段回深いとうれしいです.
ros/rosdistro#8978
e5cf7e3#diff-d0ea6af8aa6fbf40462a5c00380c59ad

これはCONTRIBUTION.mdの置き場所に関してのコメントですか?
僕のコメントに対してですか?

@k-okada

This comment has been minimized.

Copy link
Member

k-okada commented Oct 27, 2015

と追加すると,あるROSパッケージの中で$ gcとすると,↓のような感じで勝手に[${package}]を入れてくれます.

への返事です.

◉ Kei Okada

2015-10-25 22:34 GMT+09:00 Kentaro Wada notifications@github.com:

@k-okada https://github.com/k-okada

CHANGELOG.rst はパッケージ毎にできるので,実はもう一段回深いとうれしいです.
ros/rosdistro#8978 ros/rosdistro#8978
e5cf7e3
e5cf7e3
#diff-d0ea6af8aa6fbf40462a5c00380c59ad

これはCONTRIBUTION.mdの置き場所に関してのコメントですか?
僕のコメントに対してですか?


Reply to this email directly or view it on GitHub
#1202 (comment)
.

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Oct 27, 2015

[src] COMMENT

のようにするということでしょうか。

2015年10月27日火曜日、Kei Okadanotifications@github.comさんは書きました:

と追加すると,あるROSパッケージの中で$ gcとすると,↓のような感じで勝手に[${package}]を入れてくれます.

への返事です.

◉ Kei Okada

2015-10-25 22:34 GMT+09:00 Kentaro Wada <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>:

@k-okada https://github.com/k-okada

CHANGELOG.rst はパッケージ毎にできるので,実はもう一段回深いとうれしいです.
ros/rosdistro#8978 ros/rosdistro#8978
e5cf7e3
<
e5cf7e3

#diff-d0ea6af8aa6fbf40462a5c00380c59ad

これはCONTRIBUTION.mdの置き場所に関してのコメントですか?
僕のコメントに対してですか?


Reply to this email directly or view it on GitHub
<
#1202 (comment)

.


Reply to this email directly or view it on GitHub
#1202 (comment)
.

和田 健太郎 / Kentaro Wada
http://wkentaro.com

@k-okada

This comment has been minimized.

Copy link
Member

k-okada commented Oct 27, 2015

[pkg/src/hoge.l] comment
pkg comment
が理想かな

◉ Kei Okada

2015/10/27 23:51、Kentaro Wada notifications@github.com のメッセージ:

[src] COMMENT

のようにするということでしょうか。

2015年10月27日火曜日、Kei Okadanotifications@github.comさんは書きました:

と追加すると,あるROSパッケージの中で$ gcとすると,↓のような感じで勝手に[${package}]を入れてくれます.

への返事です.

◉ Kei Okada

2015-10-25 22:34 GMT+09:00 Kentaro Wada <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>:

@k-okada https://github.com/k-okada

CHANGELOG.rst はパッケージ毎にできるので,実はもう一段回深いとうれしいです.
ros/rosdistro#8978 ros/rosdistro#8978
e5cf7e3
<
e5cf7e3

#diff-d0ea6af8aa6fbf40462a5c00380c59ad

これはCONTRIBUTION.mdの置き場所に関してのコメントですか?
僕のコメントに対してですか?


Reply to this email directly or view it on GitHub
<
#1202 (comment)

.


Reply to this email directly or view it on GitHub
#1202 (comment)
.

和田 健太郎 / Kentaro Wada
http://wkentaro.com

Reply to this email directly or view it on GitHub.

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Oct 27, 2015

ファイル名入れない主義(意味でコミットする派)もあるかと思いますが、ファイル名を入れるのはどうしてでしょうか。

パッケージ名を入れるのはhistory, issueやprなどGithub上での見易さを重視したものと考えています。

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Oct 27, 2015

prefixを入れる問題点としては、一行目としてかけるコメントが少なくなってしまうことだと思います。

なので
[pkg] COMMENT
はやめて
pkg: COMMENT
にするのもあるかなと思います。

@garaemon

This comment has been minimized.

Copy link
Member Author

garaemon commented Oct 27, 2015

git logに関してファイル名は

git log --name-only --oneline

を実はしたらいい?

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Oct 27, 2015

git logに関してファイル名は
git log --name-only --oneline
を実はしたらいい?

そう考えています。"ファイル名まで見たくなるようなときはコマンドラインで見るでしょう"という過程です。
僕はgit log --statgit log filenameを使っています。

@k-okada

This comment has been minimized.

Copy link
Member

k-okada commented Oct 28, 2015

時と場合によるけど,
jsk-ros-pkg/jsk_recognition@db56da9?short_path=880d954#diff-880d954a235307b0dbf574556ead6624
において,[jsk_pcl_ros] は情報量がないですよね.自分が使っているXXノードに変更があったかなかったかわからない.

◉ Kei Okada

2015-10-28 3:13 GMT+09:00 Kentaro Wada notifications@github.com:

git logに関してファイル名は
git log --name-only --oneline
を実はしたらいい?

そう考えています。"ファイル名まで見たくなるようなときはコマンドラインで見るでしょう"という過程です。
僕はgit log --statやgit log filenameを使っています。


Reply to this email directly or view it on GitHub
#1202 (comment)
.

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Oct 28, 2015

CHANGELOG.rstを作るときにどうにかするか、commitメッセージの[pkg]はやめて[node]にするかですね。

PRやIssueだけ[pkg]付けるようにすればいいかと。

2015年10月28日水曜日、Kei Okadanotifications@github.comさんは書きました:

時と場合によるけど,

jsk-ros-pkg/jsk_recognition@db56da9?short_path=880d954#diff-880d954a235307b0dbf574556ead6624
において,[jsk_pcl_ros] は情報量がないですよね.自分が使っているXXノードに変更があったかなかったかわからない.

◉ Kei Okada

2015-10-28 3:13 GMT+09:00 Kentaro Wada <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>:

git logに関してファイル名は
git log --name-only --oneline
を実はしたらいい?

そう考えています。"ファイル名まで見たくなるようなときはコマンドラインで見るでしょう"という過程です。
僕はgit log --statやgit log filenameを使っています。


Reply to this email directly or view it on GitHub
<
https://github.com/jsk-ros-pkg/jsk_common/pull/1202#issuecomment-151594676>

.


Reply to this email directly or view it on GitHub
#1202 (comment)
.

和田 健太郎 / Kentaro Wada
http://wkentaro.com

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Oct 30, 2015

結局、コミットメッセージに関してはどうしましょうか。。

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Nov 9, 2015

ping

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Nov 15, 2015

ところで,コードの最初にBSDライセンスの文言を入れるのはルールでしょうか.それともゆるい決まりのようなものでしょうか.
.cppと.hには大体入っていますが,.pyや.lにはあまり入っていない感じだと思います.

(個人的には各ファイルのソースは短い方が好きなので,License.txtを置く方針の方が嬉しいです)

@k-okada k-okada mentioned this pull request Nov 26, 2015
@garaemon

This comment has been minimized.

Copy link
Member Author

garaemon commented Dec 4, 2015

(個人的には各ファイルのソースは短い方が好きなので,License.txtを置く方針の方が嬉しいです)

「ファイルをコピーしやすいように、ファイル全部に書いておこう」というのがwillowgarageの
スタイルでした

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Dec 4, 2015

なるほど、roscp用ですね。了解です、ありがとうございます。

@garaemon

This comment has been minimized.

Copy link
Member Author

garaemon commented Dec 4, 2015

roscpというか、自分がプログラムを書く時に、「よくわからないけど落ちてるファイルからコピーしてくる」というのをやる時にファイル事態に冗長でも書いてあるほうがライセンスの心配がない。
ということですね。

@garaemon

This comment has been minimized.

Copy link
Member Author

garaemon commented Dec 4, 2015

PRの書き方ですが、僕が[pkg/program]が好きなのはPR一覧をwebで見た時に
わかりやすい、きがするという点です

たとえば
jsk-ros-pkg/jsk_demos#1135
だと、「add box maai diff print」ではなんのことかよくわからないですが、
「drc_task_common/box-push」みたいに書いてくれたらなんとなくわかるな、
という点ですね。

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Dec 4, 2015

roscpというか、自分がプログラムを書く時に、「よくわからないけど落ちてるファイルからコピーしてくる」というのをやる時にファイル事態に冗長でも書いてあるほうがライセンスの心配がない。
ということですね。

好みの問題ですが、とりあえず、rosパッケージのファイルはそういう方針で行くということで思っておきます。

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Dec 4, 2015

コミットメッセージはどうしましょうか。

@garaemon garaemon added the Discussion label Dec 8, 2015
@k-okada

This comment has been minimized.

Copy link
Member

k-okada commented Dec 9, 2015

catkin_generate_changelog すると,

comm.txt

となります.ここから,
ros/rosdistro#10008
を出すのは結構骨が折れるので,パッケージ,ファイルの両方の情報があると嬉しいです.

wkentaro added a commit to wkentaro/jsk_common that referenced this pull request Dec 9, 2015
This is proposed by @k-okada and discussed on jsk-ros-pkg#1202

Modified:
	 jsk_tools/env-hooks/99.jsk_tools.sh
wkentaro added a commit to wkentaro/jsk_common that referenced this pull request Dec 9, 2015
This is proposed by @k-okada and discussed on jsk-ros-pkg#1202

Modified:
	 jsk_tools/env-hooks/99.jsk_tools.sh
@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Dec 9, 2015

@k-okada と話して1つの案として#1283 に落ち着いた感じです。岡田先生はこんな感じでいいそうです。 wkentaro@91eb9fe
あとはノード名を入れるかどうかですね。

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Dec 9, 2015

あと
コーディングスタイルはROSコミュニティ内のものがあるようで、
rosrun roslint cpplint *.cpp
rosrun roslint pep8 *.py
のように使えます。cpplintの方はpipでいれるられる奴にpatchが当ててありますが、pep8は同じです。

catkin run_testsでstyleまでチェックするようにしたのが #1272 ですが、
既存のファイルのスタイルを直すというのが少し手間であまり出来ていません。
roslintだけ実行したいという時は、
catkin bt --no-deps -iv --catkin-make-args roslintでできます。

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Dec 17, 2015

Could you please reopen this? @garaemon

@garaemon

This comment has been minimized.

Copy link
Member Author

garaemon commented Dec 17, 2015

wow, I closed this by mistake.

@wkentaro You can fork this PR to modify something...

@garaemon garaemon reopened this Dec 17, 2015
@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Dec 17, 2015

wow, I closed this by mistake.

Sorry, that's my fault, I mistakenly put comment Closes #{PR_NUMBER}.

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Dec 17, 2015

@wkentaro You can fork this PR to modify something...

Yes, I will.

@wkentaro

This comment has been minimized.

Copy link
Member

wkentaro commented Dec 17, 2015

sent pr garaemon#3

Describe about `jsk-commit` subcommand for `git` in CONTRIBUTION.md
@garaemon

This comment has been minimized.

Copy link
Member Author

garaemon commented Dec 17, 2015

とりあえずマージしてみます

garaemon added a commit that referenced this pull request Dec 17, 2015
Add CONTRIBUTION.md
@garaemon garaemon merged commit 6e0e93f into jsk-ros-pkg:master Dec 17, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@garaemon garaemon deleted the garaemon:contribution branch Dec 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.