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.

Sign up for GitHub

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

Jump to bottom

chore: vscode setting updates #1221

Merged
merged 1 commit into from
Apr 7, 2022
Merged

chore: vscode setting updates #1221

merged 1 commit into from
Apr 7, 2022

Conversation

alexanderbez
Copy link
Contributor

  • JSON prettyify/format
  • Add proto formatting settings

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

@alexanderbez alexanderbez mentioned this pull request Apr 7, 2022
Merged
4 tasks
ValarDragon
ValarDragon approved these changes Apr 7, 2022
View reviewed changes
Copy link
Member

@ValarDragon ValarDragon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I feel like this won't work out of the box on Linux systems that don't have clang installed, but I believe this should be fine since vscode shouldn't error if it doesn't exist.

I'll see what happens on my machine without clang

@alexanderbez
Copy link
Contributor Author

Yeah so you need clang-format installed, that is the caveat, but I think it's worth it for consistent proto formatting. WDYT?

@ValarDragon
Copy link
Member

I'll install it locally (clang was actually on my setup todo list), I'm just hesistant about making it a required step vs a sub-component of some automated commands, since its another thing to add to the complexities of new dev's setting up a working environment. (It also gets fixed in every make proto-format call)

Adding it to the default settings.json is the least controversial thing though, since thats a "if its installed, use it in the way we prefer" situation, which is ideal!

@ValarDragon ValarDragon merged commit 10b318e into main Apr 7, 2022
@ValarDragon ValarDragon deleted the bez/vscode-settings-lint branch April 7, 2022 21:19
p0mvn
p0mvn reviewed Apr 7, 2022
View reviewed changes
.vscode/settings.json
"tabnine.experimentalAutoImports": true,
"go.lintTool": "golangci-lint",
"go.formatTool": "gofumports",
"go.testOnSave": true,
Copy link
Member

Choose a reason for hiding this comment

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

I'm personally used to working with auto-save and just noticed the following:
image

Can we let everyone decide if they would like this enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, wanna make a followup PR?

Copy link
Member

Choose a reason for hiding this comment

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

On it

@p0mvn p0mvn mentioned this pull request Apr 7, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@p0mvn p0mvn p0mvn left review comments

@ValarDragon ValarDragon ValarDragon approved these changes

@mattverse mattverse Awaiting requested review from mattverse

Assignees
No one assigned
Labels
None yet
Projects
Osmosis Chain Development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alexanderbez @ValarDragon @p0mvn