add sdkch to maintain changelog effectively and free of file conflicts #3828

alessio · 2019-03-08T04:14:03Z

Imported from https://github.com/alessio/modcl

Closes: #2380

Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (docs/)
Added entries in PENDING.md with issue #
rereviewed Files changed in the github PR explorer

For Admin Use:

Added appropriate labels to PR (ex. wip, ready-for-review, docs)
Reviewers Assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

codecov · 2019-03-08T14:07:13Z

Codecov Report

Merging #3828 into develop will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #3828      +/-   ##
===========================================
+ Coverage    60.94%   60.95%   +0.01%     
===========================================
  Files          192      192              
  Lines        14360    14360              
===========================================
+ Hits          8751     8753       +2     
+ Misses        5035     5033       -2     
  Partials       574      574

alexanderbez

This should belong under cmd/sdkch. It should also include a simple README.md markdown that explains a basic synopsis and general usage. (e.g. I have no idea what this is or how to use it, although I have a general idea based of the simple help).

Also, missing a pending log entry.

rigelrozanski · 2019-03-08T23:15:25Z

@alexanderbez I actually think this makes sense not to include this tool in cmd/ and also to never run it from a binary (but always with go run from the makefile) - I think of it more of a bash script as opposed to a something we want to actively keep in the $PATH

alessio · 2019-03-08T23:18:41Z

@rigelrozanski the only the thing that could be automated in the Makefile is the generation of the Markdown changelog. For all the rest, I'd warmly recommend to compile it locally like all the other tools.

rigelrozanski · 2019-03-09T02:10:28Z

yeah it's not a strong preference by any means, having it in cmd sounds reasonable too - I just don't want to clutter cmd/ but maybe nbd

alessio · 2019-03-09T20:03:37Z

@alexanderbez I don't mind moving it under cmd.

alexanderbez · 2019-03-09T21:17:44Z

If its a command or ran like one (which it is -- even though it's a tool), it should live in cmd/. cmd/ is meant to be populated with such commands.

alexanderbez

Looks great @alessio!. I left a small remark. Also, shouldn't we update our contribution doc? All contributing devs need to use this flow now.

cmd/sdkch/main.go

cmd/sdkch/README.md

rigelrozanski

lot's of good work, but also lots of changes which should be addressed before this is merged. Please read through all my comments before addressing (a few of the comments supercede each other a bit)

cmd/sdkch/README.md

cmd/sdkch/main.go

cmd/sdkch/README.md

cmd/sdkch/main.go

cmd/sdkch/README.md

alessio · 2019-03-12T10:46:13Z

@alexanderbez dixit:

Also, shouldn't we update our contribution doc?

I think we should update the PR template, there is nothing to update in CONTRIBUTING.md

cwgoes · 2019-03-12T14:58:33Z

Great tooling addition!

A few notes:

Fails the linter
Can sdkch just prepend to CHANGELOG.md by default?
Can we ensure -prune also works when specified after the command?

alessio · 2019-03-12T15:08:55Z

@cwgoes comments were addressed in person, nolint: errcheck has been reintroduced

alessio · 2019-03-12T15:59:35Z

@cwgoes @rigelrozanski @alexanderbez all comments have been addressed. Please review again

rigelrozanski

Some more things to address

~~installation doesn't seem to be working from make devtools even after make devtools_clean~~
when attempting an add I got the error sdkch: couldn't open an editor: exec: "editor": executable file not found in $PATH
I would mention in this error maybe here that the $EDITOR environment variable can be set to achieve the same thing (which is what I ended up using)
during adding, what's with the first slash here?? shouldn't it be [\#1543] instead of \[\#1543]
* \[\#1543](https://github.com/cosmos/cosmos-sdk/issues/1543) whuats up
I'm confused as to what -prune is supposed to do (doesn't seem to do much)
~~add sdkch to maintain changelog effectively and free of file conflicts #3828 (comment)~~
~~add sdkch to maintain changelog effectively and free of file conflicts #3828 (comment)~~
~~need to add new instructions within .github/PULL_REQUEST_TEMPLATE.md~~

alessio · 2019-03-13T10:17:44Z

@rigelrozanski

installation doesn't seem to be working from make devtools even after make devtools_clean

Makefile was buggy, fixed now:

alessio@bangalter:~/.../cosmos/cosmos-sdk$ which sdkch
alessio@bangalter:~/.../cosmos/cosmos-sdk$ make devtools-clean
cd /home/alessio/work/tendermint/bin && rm -f dep gometalinter statik goimports
rm devtools-stamp
alessio@bangalter:~/.../cosmos/cosmos-sdk$ make devtools
mkdir -p /home/alessio/work/tendermint/src/github.com/golang && (test ! -d /home/alessio/work/tendermint/src/github.com/golang/dep && cd /home/alessio/work/tendermint/src/github.com/golang && git clone https://github.com/golang/dep) || true &&  cd /home/alessio/work/tendermint/src/github.com/golang/dep && git fetch origin && git checkout -q 22125cfaa6ddc71e145b1535d4b7ee9744fefff2
cd /home/alessio/work/tendermint/src/github.com/golang/dep/cmd/dep && /usr/local/bin/go install
mkdir -p /home/alessio/work/tendermint/src/github.com/alecthomas && (test ! -d /home/alessio/work/tendermint/src/github.com/alecthomas/gometalinter && cd /home/alessio/work/tendermint/src/github.com/alecthomas && git clone https://github.com/alecthomas/gometalinter) || true &&  cd /home/alessio/work/tendermint/src/github.com/alecthomas/gometalinter && git fetch origin && git checkout -q v3.0.0 && cd /home/alessio/work/tendermint/src/github.com/alecthomas/gometalinter && /usr/local/bin/go install
mkdir -p /home/alessio/work/tendermint/src/github.com/rakyll && (test ! -d /home/alessio/work/tendermint/src/github.com/rakyll/statik && cd /home/alessio/work/tendermint/src/github.com/rakyll && git clone https://github.com/rakyll/statik) || true &&  cd /home/alessio/work/tendermint/src/github.com/rakyll/statik && git fetch origin && git checkout -q v0.1.5 && cd /home/alessio/work/tendermint/src/github.com/rakyll/statik && /usr/local/bin/go install
go get golang.org/x/tools/cmd/goimports
--> Downloading linters (this may take awhile)
/home/alessio/work/tendermint/src/github.com/alecthomas/gometalinter/scripts/install.sh -b /home/alessio/work/tendermint/bin
alecthomas/gometalinter info checking GitHub for latest tag
alecthomas/gometalinter info found version: 3.0.0 for v3.0.0/linux/amd64
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/gometalinter
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/gocyclo
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/nakedret
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/misspell
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/gosec
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/golint
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/ineffassign
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/goconst
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/errcheck
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/maligned
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/unconvert
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/dupl
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/structcheck
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/varcheck
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/safesql
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/deadcode
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/lll
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/goimports
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/gotype
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/staticcheck
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/interfacer
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/unparam
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/gochecknoinits
alecthomas/gometalinter info installed /home/alessio/work/tendermint/bin/gochecknoglobals
go get github.com/tendermint/lint/golint
go install ./cmd/sdkch
touch devtools-stamp
alessio@bangalter:~/.../cosmos/cosmos-sdk$ which sdkch
/home/alessio/work/tendermint/bin/sdkch

when attempting an add I got the error sdkch: couldn't open an editor: exec: "editor": executable file not found in $PATH
I would mention in this error maybe here that the $EDITOR environment variable can be set to achieve the same thing (which is what I ended up using)

Good one. Fixed.

during adding, what's with the first slash here?? shouldn't it be [Will Basecoin example support staking and slashing #1543] instead of [Will Basecoin example support staking and slashing #1543]

sdkch generate does the whole thing for you. When you add a new entry just use the #nnn syntax, don't prepend it with slashes or square brackets whatsoever. Just write #5643 whuats up

I'm confused as to what -prune is supposed to do (doesn't seem to do much)

Delete all files under .pending. It does not delete directories.

add sdkch to maintain changelog effectively and free of file conflicts #3828 (comment)

Responded inline. I don't understand what make cl should do, usage requirements are a bit unclear to me.

add sdkch to maintain changelog effectively and free of file conflicts #3828 (comment)

Responded inline. Fixed.

need to add new instructions within .github/PULL_REQUEST_TEMPLATE.md

Done

sabau

I gave a try and seems to to the job with vim

alexanderbez

TestedACK

Also, I'm not sure how I feel about the file names. Some messages can be somewhat long. What are your thoughts on taking a -f/--file flag?

Makefile

.github/PULL_REQUEST_TEMPLATE.md

cmd/sdkch/README.md

alessio · 2019-03-13T14:41:12Z

I'll address your make cl in an ensuing PR

cwgoes

tACK, I think @rigelrozanski should give a final review though

cwgoes · 2019-03-13T14:44:40Z

(presumably we want release process integration, but that can happen separately)

rigelrozanski

pushed a minor cleanup. Looks good, thanks for going through all the feedback!

p.s. looks like it need a pending update before merging!

Closes: #2380

cwgoes

The PENDING.md update is in the new format!

ACK, thanks @alessio

alessio requested review from jaekwon, sabau, alexanderbez, jackzampolin, rigelrozanski and cwgoes March 8, 2019 04:14

alessio requested a review from ebuchman as a code owner March 8, 2019 04:14

alessio changed the title ~~add sdkch to maintain PENDING.md effectively and free of file conflicts~~ add sdkch to maintain changelog effectively and free of file conflicts Mar 8, 2019

alessio force-pushed the alessio/modular-changelog branch 2 times, most recently from 24f3d84 to 50c662f Compare March 8, 2019 15:48

alexanderbez suggested changes Mar 8, 2019

View reviewed changes

alexanderbez added ready-for-review tooling dev tooling within the sdk labels Mar 8, 2019

alessio requested a review from alexanderbez March 8, 2019 23:38

alessio force-pushed the alessio/modular-changelog branch 2 times, most recently from 18f83f4 to e8b2a49 Compare March 10, 2019 00:40

alexanderbez reviewed Mar 11, 2019

View reviewed changes

cmd/sdkch/main.go Show resolved Hide resolved

cmd/sdkch/README.md Outdated Show resolved Hide resolved

rigelrozanski suggested changes Mar 12, 2019

View reviewed changes

alessio requested review from mossid, alexanderbez and liamsi March 12, 2019 10:46

alessio force-pushed the alessio/modular-changelog branch 3 times, most recently from b0a4bd7 to 0ede4b4 Compare March 12, 2019 15:38

alessio requested a review from rigelrozanski March 12, 2019 15:38

alessio force-pushed the alessio/modular-changelog branch from 0ede4b4 to c8180a7 Compare March 12, 2019 15:58

rigelrozanski suggested changes Mar 13, 2019

View reviewed changes

alessio requested a review from rigelrozanski March 13, 2019 10:22

alessio force-pushed the alessio/modular-changelog branch 2 times, most recently from b1ca3e5 to a0ef8da Compare March 13, 2019 10:45

sabau approved these changes Mar 13, 2019

View reviewed changes

alexanderbez suggested changes Mar 13, 2019

View reviewed changes

Makefile Show resolved Hide resolved

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved

cmd/sdkch/README.md Outdated Show resolved Hide resolved

cmd/sdkch/README.md Show resolved Hide resolved

cmd/sdkch/README.md Outdated Show resolved Hide resolved

alexanderbez approved these changes Mar 13, 2019

View reviewed changes

alessio force-pushed the alessio/modular-changelog branch from f30c2fb to 4e90cef Compare March 13, 2019 14:32

cwgoes approved these changes Mar 13, 2019

View reviewed changes

rigelrozanski approved these changes Mar 13, 2019

View reviewed changes

add sdkch to maintain PENDING.md effectively and free of file conflicts

39bbe73

Closes: #2380

alessio force-pushed the alessio/modular-changelog branch from c5a96ae to 39bbe73 Compare March 14, 2019 12:10

cwgoes approved these changes Mar 14, 2019

View reviewed changes

cwgoes merged commit 648b432 into develop Mar 14, 2019

cwgoes deleted the alessio/modular-changelog branch March 14, 2019 13:25

This was referenced Mar 14, 2019

Remove Shares Concept from Unbond/Redelegate UX #3857

Merged

Return errors instead of panic #3782

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add sdkch to maintain changelog effectively and free of file conflicts #3828

add sdkch to maintain changelog effectively and free of file conflicts #3828

alessio commented Mar 8, 2019 •

edited

Loading

codecov bot commented Mar 8, 2019 •

edited

Loading

alexanderbez left a comment •

edited

Loading

rigelrozanski commented Mar 8, 2019

alessio commented Mar 8, 2019

rigelrozanski commented Mar 9, 2019

alessio commented Mar 9, 2019

alexanderbez commented Mar 9, 2019 •

edited

Loading

alexanderbez left a comment

rigelrozanski left a comment •

edited

Loading

alessio commented Mar 12, 2019

cwgoes commented Mar 12, 2019

alessio commented Mar 12, 2019

alessio commented Mar 12, 2019

rigelrozanski left a comment •

edited

Loading

alessio commented Mar 13, 2019

sabau left a comment

alexanderbez left a comment •

edited

Loading

alessio commented Mar 13, 2019

cwgoes left a comment

cwgoes commented Mar 13, 2019

rigelrozanski left a comment

cwgoes left a comment

add sdkch to maintain changelog effectively and free of file conflicts #3828

add sdkch to maintain changelog effectively and free of file conflicts #3828

Conversation

alessio commented Mar 8, 2019 • edited Loading

codecov bot commented Mar 8, 2019 • edited Loading

Codecov Report

alexanderbez left a comment • edited Loading

Choose a reason for hiding this comment

rigelrozanski commented Mar 8, 2019

alessio commented Mar 8, 2019

rigelrozanski commented Mar 9, 2019

alessio commented Mar 9, 2019

alexanderbez commented Mar 9, 2019 • edited Loading

alexanderbez left a comment

Choose a reason for hiding this comment

rigelrozanski left a comment • edited Loading

Choose a reason for hiding this comment

alessio commented Mar 12, 2019

cwgoes commented Mar 12, 2019

alessio commented Mar 12, 2019

alessio commented Mar 12, 2019

rigelrozanski left a comment • edited Loading

Choose a reason for hiding this comment

alessio commented Mar 13, 2019

sabau left a comment

Choose a reason for hiding this comment

alexanderbez left a comment • edited Loading

Choose a reason for hiding this comment

alessio commented Mar 13, 2019

cwgoes left a comment

Choose a reason for hiding this comment

cwgoes commented Mar 13, 2019

rigelrozanski left a comment

Choose a reason for hiding this comment

cwgoes left a comment

Choose a reason for hiding this comment

alessio commented Mar 8, 2019 •

edited

Loading

codecov bot commented Mar 8, 2019 •

edited

Loading

alexanderbez left a comment •

edited

Loading

alexanderbez commented Mar 9, 2019 •

edited

Loading

rigelrozanski left a comment •

edited

Loading

rigelrozanski left a comment •

edited

Loading

alexanderbez left a comment •

edited

Loading