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

Document our conventions for writing messages #13916

Merged
merged 6 commits into from Apr 4, 2023

Conversation

imply-cheddar
Copy link
Contributor

This adds some documentation for our message-writing conventions.

@clintropolis clintropolis added the Area - Dev For items related to the project itself, like dev docs and checklists, but not CI label Mar 10, 2023
@abhishekagarwal87
Copy link
Contributor

LGTM.

Copy link
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

Looks good to me as-is. Had a couple of questions that don't block my approval. Thanks for starting this off!

dev/style-conventions.md Show resolved Hide resolved
dev/style-conventions.md Outdated Show resolved Hide resolved
dev/style-conventions.md Outdated Show resolved Hide resolved
dev/style-conventions.md Outdated Show resolved Hide resolved
@imply-cheddar
Copy link
Contributor Author

@paul-rogers @gianm both of you left comments that I have addressed with updates to the text. Just pinging to give you a chance to double check the adjustments and leave more comments if you do not feel like the new explanations make sense.

The thing I'm mostly wondering about is the explanation about [] and it not being the equivalent of quotes.

@paul-rogers
Copy link
Contributor

@imply-cheddar, error messages are for end users. Even when they are for sys admins or devs, they go to end users first. As an end user of many products, I find column[foo] to be odd and un-English like. There may well be a good technical reason, but since our audience is end users, and they don't know that technical goal, we should use normally accepted English practice. That should include quotes, but if we choose to use brackets in place of quotes, at least use common spacing.

That said, the code works fine without spaces and users can get used to Druid style. So, LGTM.

@cheddar cheddar merged commit 232491e into apache:master Apr 4, 2023
8 checks passed
@clintropolis clintropolis added this to the 26.0 milestone Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area - Dev For items related to the project itself, like dev docs and checklists, but not CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants