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

Naming things: expand language around the use of common words, brevity, wide consultation, and future-proofing. #163

Merged
merged 4 commits into from
May 6, 2020
Merged

Naming things: expand language around the use of common words, brevity, wide consultation, and future-proofing. #163

merged 4 commits into from
May 6, 2020

Conversation

hober
Copy link
Contributor

@hober hober commented Mar 4, 2020

No description provided.

@hober hober added the Status: In Progress We're working on it but ideas not fully formed yet. label Mar 4, 2020
@hober hober assigned hober and alice Mar 4, 2020
@hober hober requested review from dbaron and cynthia March 5, 2020 00:31
@torgo torgo self-requested a review March 11, 2020 21:08
hober
hober commented Mar 17, 2020
View reviewed changes
index.bs Show resolved Hide resolved
@hober
Copy link
Contributor Author

hober commented Mar 18, 2020

Okay, this has been rebased onto @atanassov's squashed merge. Please review.

@hober hober requested a review from atanassov March 18, 2020 17:30
@hober hober removed the Status: In Progress We're working on it but ideas not fully formed yet. label Mar 18, 2020
atanassov
atanassov suggested changes Mar 18, 2020
View reviewed changes
index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Show resolved Hide resolved
annevk
annevk reviewed Mar 19, 2020
View reviewed changes
index.bs Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Outdated
Keep in mind that most web developers are not native English speakers.
Whenever possible, names should be chosen that use common vocabulary
non-native English speakers are likely to understand when first encountering the name.
Always value readability over brevity.
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 not sure I entirely agree with this. E.g., how would this judge fetch() or response.json()?

Copy link
Contributor

Choose a reason for hiding this comment

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

Applying any or all of these rules retrospectively may not necessary work. In your example I'd argue that fetch() is a common vocabulary for getting things on one's behalf. On the other hand, json is a well known reference similar to style, css etc. that shouldn't need a long form... if the format isn't known to the user it should be trivial to find of course.

Copy link
Member

Choose a reason for hiding this comment

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

Well, it might not be clear that it json() consumes the body of the response though. That's more the thing I was getting it. But also, I think that there's value in brevity as it's never possible to encapsulate the full complexity in the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tried to capture this in d59b3a7, @atanassov & @annevk.

@dbaron dbaron linked an issue Mar 30, 2020 that may be closed by this pull request
expand on consistency of naming #149
Closed
dbaron
dbaron approved these changes Apr 1, 2020
View reviewed changes
Copy link
Member

@dbaron dbaron left a comment

Choose a reason for hiding this comment

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

Seems good to me.

(My one comment here is a reply to @annevk's comment, not a comment on the actual diff.)

index.bs Show resolved Hide resolved
@torgo torgo added this to the 2020-05-04-week milestone Apr 1, 2020
@dbaron dbaron mentioned this pull request Apr 8, 2020
Closed
@hober hober force-pushed the alice-tess-149 branch from 65012b8 to b34a75e Compare May 4, 2020 21:11
@hober hober force-pushed the alice-tess-149 branch from b34a75e to 1da4c4a Compare May 4, 2020 21:12
@hober hober requested review from atanassov and annevk May 4, 2020 21:43
@hober hober changed the title WIP: Attempt to address some of the naming feedback in #132 and #149. Attempt to address some of the naming feedback in #132 and #149. May 4, 2020
@hober hober linked an issue May 4, 2020 that may be closed by this pull request
Discourage fancy words in names #132
Closed
@hober hober changed the title Attempt to address some of the naming feedback in #132 and #149. Naming things: expand language around the use of common words, brevity, wide consultation, and future-proofing. May 4, 2020
@hober hober force-pushed the alice-tess-149 branch from 9f279d2 to d59b3a7 Compare May 4, 2020 21:49
atanassov
atanassov approved these changes May 4, 2020
View reviewed changes
Copy link
Contributor

@atanassov atanassov left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my issues. LGTM

alice
alice approved these changes May 4, 2020
View reviewed changes
index.bs Show resolved Hide resolved
dbaron
dbaron approved these changes May 4, 2020
View reviewed changes
Copy link
Member

@dbaron dbaron left a comment

Choose a reason for hiding this comment

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

The changes in the three new commits all look good to me.

@hober hober removed request for cynthia and torgo May 4, 2020 23:32
@hober
Copy link
Contributor Author

hober commented May 4, 2020

@annevk, could you take another look? Thanks!

annevk
annevk approved these changes May 5, 2020
View reviewed changes
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I think overall this is good and you should land it.

Having read https://adactio.com/journal/16811 recently I do wonder if at some point we should make it more clear what the tradeoffs are and what to prioritize when making one.

@torgo torgo merged commit c910f19 into master May 6, 2020
@torgo
Copy link
Member

torgo commented May 6, 2020

Approved on call 6 May 2020

@hober hober deleted the alice-tess-149 branch May 7, 2020 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atanassov atanassov atanassov approved these changes

@annevk annevk annevk approved these changes

@torgo torgo torgo approved these changes

@dbaron dbaron dbaron approved these changes

@alice alice alice approved these changes

Assignees

@hober hober

@alice alice

Labels
None yet
Projects
None yet
Milestone
2020-05-04-week
Development

Successfully merging this pull request may close these issues.

expand on consistency of naming Discourage fancy words in names
6 participants
@hober @torgo @alice @dbaron @annevk @atanassov