Skip to content

Conversation

@fadeev
Copy link
Contributor

@fadeev fadeev commented Sep 7, 2022

Noticed we're using both dashes - and slashes / for key index separators. Might as well stick to one convention.

@fadeev fadeev changed the title fix: use slashes for key separators fix(template): use slashes for key separators Sep 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2022

Visit the preview URL for this PR (updated for commit 9d91afc):

https://ignite-go-docs--pr2815-fadeev-key-sep-k02h8rxr.web.app

(expires Wed, 14 Sep 2022 14:49:31 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Copy link
Member

@jeronimoalbi jeronimoalbi left a comment

Choose a reason for hiding this comment

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

Personally I like the path notation with slashes way more than the dashed one but I do wonder if there is a general consensus on using it over the previous dashed notation. In any case LGTM 👍

Copy link
Contributor

@tbruyelle tbruyelle left a comment

Choose a reason for hiding this comment

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

LGTM!

@lumtis
Copy link
Contributor

lumtis commented Sep 7, 2022

Personally I like the path notation with slashes way more than the dashed one but I do wonder if there is a general consensus on using it over the previous dashed notation. In any case LGTM 👍

We used dashes initially and switched to slashes which is more intuitive for a path and used in SDK modules. I didn't know some dashes remained

@jeronimoalbi jeronimoalbi added the skip-changelog Don't check changelog for new entries label Sep 7, 2022
@aljo242 aljo242 merged commit 5e4c92e into develop Sep 7, 2022
@aljo242 aljo242 deleted the fadeev/key-sep branch September 7, 2022 15:24
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
* fix: use slashes for key separators

* docs: prefixes

Co-authored-by: Jerónimo Albi <jeronimo.albi@tendermint.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Don't check changelog for new entries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants