Skip to content

[NFC][libclc] Code readability improvements #8230

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
Merged
merged 2 commits into from
Feb 10, 2023
Merged

[NFC][libclc] Code readability improvements #8230

merged 2 commits into from
Feb 10, 2023

Conversation

ldrumm
Copy link
Contributor

@ldrumm ldrumm commented Feb 7, 2023

  • Hoist some repeated long subexpressions into named variables.
  • Use some shorter (but hopefully still readable) variables names where appropriate
  • Use auto for the result of a static_cast to avoid needless repetition
  • contract some diagnostic printing.
  • split a long function

@ldrumm ldrumm requested a review from a team as a code owner February 7, 2023 16:45
@ldrumm ldrumm requested a review from sergey-semenov February 7, 2023 16:45
@ldrumm ldrumm temporarily deployed to aws February 7, 2023 17:11 — with GitHub Actions Inactive
bader
bader requested changes Feb 8, 2023
View reviewed changes
@ldrumm ldrumm temporarily deployed to aws February 8, 2023 22:47 — with GitHub Actions Inactive
ldrumm
ldrumm commented Feb 9, 2023
View reviewed changes
Copy link
Contributor Author

@ldrumm ldrumm 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 the review. I'll update

@ldrumm
[NFC][libclc] Code readability improvements
57a8400 
- Hoist some repeated long subexpressions into named variables.
- Use some shorter (but hopefully still readable) variables names where
  appropriate
- Use `auto` for the result of a `static_cast` to avoid needless
  repetition
- contract some diagnostic printing.
- split a long function
@ldrumm ldrumm temporarily deployed to aws February 9, 2023 12:04 — with GitHub Actions Inactive
@ldrumm ldrumm temporarily deployed to aws February 9, 2023 15:58 — with GitHub Actions Inactive
@bader
Copy link
Contributor

bader commented Feb 9, 2023

@ldrumm, please, be merciful for the reviewers and don't use force-push.

@ldrumm
Copy link
Contributor Author

ldrumm commented Feb 9, 2023 •
edited
Loading

@ldrumm, please, be merciful for the reviewers and don't use force-push.

Sorry. I'm used to gitlab which does this completely seamlessly. github seems to get very confused.

@ldrumm ldrumm temporarily deployed to aws February 9, 2023 17:33 — with GitHub Actions Inactive
@ldrumm ldrumm temporarily deployed to aws February 9, 2023 20:04 — with GitHub Actions Inactive
@bader bader merged commit fdf63c4 into intel:sycl Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bader bader bader approved these changes

@sergey-semenov sergey-semenov sergey-semenov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ldrumm @bader @sergey-semenov