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

style: use an indent of 2 spaces #38

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

jayaseelan-james
Copy link
Contributor

What does this Pull Request accomplish?

This updates the indentation of all the proto files in this repo to use 2 spaces based on this comment from @jasonmreding.

Consequently, a follow-up PR will be raised updating the proto copies in measurementlink-labview and measurementlink-python repositories.

Why should this Pull Request be merged?

To make sure our proto files follow the general file formatting standards.

What testing has been done?

NA.

@jayaseelan-james
Copy link
Contributor Author

Now after reading this Style Guide, I'm noticing that most of the comments in our proto files are spread too horizontally, often crossing the column length of 100. I'd like to inquire if it's appropriate to raise a follow-up PR encompassing them. I'm uncertain about its potential benefits, so I'd like to gauge the interest of the owners before proceeding with the work.

@jasonmreding
Copy link
Collaborator

Now after reading this Style Guide, I'm noticing that most of the comments in our proto files are spread too horizontally, often crossing the column length of 100. I'd like to inquire if it's appropriate to raise a follow-up PR encompassing them. I'm uncertain about its potential benefits, so I'd like to gauge the interest of the owners before proceeding with the work.

I've always viewed line lengths in coding conventions as more of a suggestion than a rule. The goal is to have the code/comments be readable on the majority of modern monitors without having to scroll. Having a line length limit of 80 seems like a rule that was more appropriate twenty years ago. Most modern development setups are probably capable of displaying at least twice that horizontal space if not triple without scrolling. Our own C# coding conventions typically use 150 as the limit, and even then that isn't a hard limit. My preference would be to keep things in the same ballpark and not take any action unless there is something egregious. Looking around, most things looks reasonable to me except for perhaps pin_map_service.proto. There are some comments in there that seem a little excessive in regards to line length.

@jayaseelan-james jayaseelan-james merged commit d618995 into main Mar 5, 2024
1 check passed
@jayaseelan-james jayaseelan-james deleted the users/jay/format-proto-files branch March 5, 2024 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants