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

Web: rename generic variable name to a helpful name #47722

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Conversation

kimlisa
Copy link
Contributor

@kimlisa kimlisa commented Oct 18, 2024

part of #46742

as a result of comment: #47345 (comment)

separated to reduce noise

Copy link
Member

Choose a reason for hiding this comment

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

Continuing on the topic started in my comment (#47347 (comment)), this is a good start and it addresses the point I had about data being a vague term. Long term we'd need to address things like
AccessRequestCheckout receiving unfiltered data and then filtering it again.

What I was getting onto in that comment is that there are concepts in the code waiting to be discovered and given names. Resorting to using terms like "data" means that the next person reading that code will have to figure out what's what by themselves.

I'm yet to use the actual UI you've built in those PRs myself, but from reading the code, it seems that there are some items within pendingAccessRequests that are meant to be used when rendered on screen and some that are meant to be used when communicating with the backend. If that's the case, then those items are good examples of two specific concepts for which we could find specific names. Sometimes prepending a variable name with an adjective (e.g. data and filteredData) can only do so much and we have invent new concepts to move forward.

@kimlisa kimlisa added this pull request to the merge queue Oct 22, 2024
Merged via the queue into master with commit f8bcf4b Oct 22, 2024
41 of 44 checks passed
@kimlisa kimlisa deleted the lisa/better-name branch October 22, 2024 17:30
@public-teleport-github-review-bot

@kimlisa See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Failed

github-merge-queue bot pushed a commit that referenced this pull request Nov 4, 2024
* Web: rename generic variable name to a helpful name (#47722)

* Update snap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v15 backport/branch/v16 no-changelog Indicates that a PR does not require a changelog entry size/sm ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants