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

Make naming of CodeGenerator variable consistent #5594

Closed
0xdaryl opened this issue Oct 3, 2020 · 7 comments · Fixed by #6990
Closed

Make naming of CodeGenerator variable consistent #5594

0xdaryl opened this issue Oct 3, 2020 · 7 comments · Fixed by #6990

Comments

@0xdaryl
Copy link
Contributor

0xdaryl commented Oct 3, 2020

The overwhelmingly common short form for a variable to store the TR::CodeGenerator object is cg (about 30000 instances). The next most common form is codeGen (about 400). We should use a single form for consistency and given the popularity of cg that should be the preferred name. Sweep through the OMR compiler and make that change.

Here is a pipeline of greps to help find some of the places to change:

grep -rn codeGen * | egrep -v 'codeGen[a-zA-Z0-9(]'
@pkrc267
Copy link

pkrc267 commented Oct 12, 2020

I would like to work on this. Can you please assign it to me?

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Oct 12, 2020

Thanks for your offer to help!

@pkrc267
Copy link

pkrc267 commented Oct 13, 2020

Thankyou so much 🙂

A quick question. After renaming all the instances, should I build and test the whole project?

@0xdaryl
Copy link
Contributor Author

0xdaryl commented Oct 13, 2020

Generally, it is a good practice to test your changes to the extent that you can on your own before making a pull request (for example, on x86 Linux). The community certainly understands that you can't test them on every platform and architecture combination.

When you do make a PR (or any time that you push new commits), a Travis CI job will run automatically that will perform a build (x86 Linux only). Once a committer has reviewed your code, they will run a much broader set of sanity tests on all architectures the OpenJ9 community cares about that you will see in the "Checks" section of the PR.

@pkrc267
Copy link

pkrc267 commented Oct 14, 2020

Made the names consistent and made a PR.
@0xdaryl Please give feedback if I need to make certain things better or do anything different.

pkrc267 added a commit to pkrc267/omr that referenced this issue Nov 7, 2020
This patch makes all instances of CodeGenerator variables from `codeGen` to `cg`.

Issue: eclipse#5594
Signed-off-by: Prakhar Yadav <[email protected]>
pkrc267 added a commit to pkrc267/omr that referenced this issue Nov 7, 2020
Resolving CodeGenerator object allocation issue.
`CodeGenerator *cg = cg()`
to
`CodeGenerator *cg = this->cg()`

Issue: eclipse#5594
Signed-off-by: Prakhar Yadav <[email protected]>
pkrc267 added a commit to pkrc267/omr that referenced this issue Nov 8, 2020
Update SystemLinkageLinux.cpp line 409 `cg()` to `this->cg()`

Issue: eclipse#5594
Signed-off-by: Prakhar Yadav <[email protected]>
pkrc267 added a commit to pkrc267/omr that referenced this issue Nov 9, 2020
This patch makes all instances of CodeGenerator variables
from `codeGen` to `cg`, and resolves issues of object allocation
by replacing
`CodeGenerator *cg = cg()`
to
`CodeGenerator *cg = this->cg()`

Issue: eclipse#5594
Signed-off-by: Prakhar Yadav <[email protected]>
@0xdaryl
Copy link
Contributor Author

0xdaryl commented May 2, 2023

As there has been no indication of progress on this issue I will un-assign it. I appreciate your offer to help the project. You are welcome to pick up this issue (if it is still open) or any other issue should you wish to contribute to Eclipse OMR in the future.

@0xdaryl
Copy link
Contributor Author

0xdaryl commented May 2, 2023

@dylanjtuttle : please have a look at this

dylanjtuttle added a commit to dylanjtuttle/omr that referenced this issue May 11, 2023
Make naming of variables holding the TR::CodeGenerator object
consistent by renaming variables named codeGen to cg

Issue: eclipse#5594
Signed-off-by: Dylan Tuttle <[email protected]>
dylanjtuttle added a commit to dylanjtuttle/omr that referenced this issue May 18, 2023
Make naming of variables holding the TR::CodeGenerator object
consistent by renaming variables named codeGen to cg

Closes: eclipse#5594
Signed-off-by: Dylan Tuttle <[email protected]>
dylanjtuttle added a commit to dylanjtuttle/omr that referenced this issue Jun 6, 2023
Make naming of variables holding the TR::CodeGenerator object
consistent by renaming variables named codeGen to cg

Closes: eclipse#5594
Signed-off-by: Dylan Tuttle <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants