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

refac: rename group internal operations #391

Merged
merged 5 commits into from
Apr 18, 2024
Merged

refac: rename group internal operations #391

merged 5 commits into from
Apr 18, 2024

Conversation

chokobole
Copy link
Contributor

@chokobole chokobole commented Apr 16, 2024 •
edited

Description

This PR renames some group operations after the internal discussion.

  • DoDoubleXXX() to DoubleImplXXX()
  • DoSquareXXX() to SquareImplXXX()
  • Negative() to Negate()
  • NegInPlace() to NegateInPlace()

@chokobole chokobole force-pushed the refac/rename-group-internal-operations branch from 8803136 to 8383ee4 Compare April 16, 2024 14:37
@chokobole chokobole marked this pull request as ready for review April 16, 2024 14:37
@chokobole chokobole force-pushed the refac/rename-group-internal-operations branch from 746d7a5 to 6d51eaa Compare April 17, 2024 12:10
Insun35
Insun35 approved these changes Apr 17, 2024
View reviewed changes
Copy link
Contributor

@Insun35 Insun35 left a comment

Choose a reason for hiding this comment

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

LGTM

TomTaehoonKim
TomTaehoonKim approved these changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@TomTaehoonKim TomTaehoonKim left a comment

Choose a reason for hiding this comment

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

LGTM

dongchangYoo
dongchangYoo approved these changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@dongchangYoo dongchangYoo left a comment

Choose a reason for hiding this comment

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

LGTM

ashjeong
ashjeong reviewed Apr 18, 2024
View reviewed changes
Copy link
Contributor

@ashjeong ashjeong left a comment

Choose a reason for hiding this comment

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

0c88845
We decided to use full name for the unary operations.
->
We decided to use the full name for unary operations.

9481472
I think you missed rational_field.h

9481472 & 6d51eaa
This is for the consistency with other operation that has DoXXX as
a private method.
->
This keeps consistency with other operations that have DoXXX as
a private method.

@chokobole chokobole force-pushed the refac/rename-group-internal-operations branch from 6d51eaa to 5c7a951 Compare April 18, 2024 08:08
@ashjeong
Copy link
Contributor

0c88845 We decided to use full name for the unary operations. -> We decided to use the full name for unary operations.

9481472 I think you missed rational_field.h

9481472 & 6d51eaa This is for the consistency with other operation that has DoXXX as a private method. -> This keeps consistency with other operations that have DoXXX as a private method.

I realized I missed the same error for f65dd0b
use full name for the unary operations. -> We decided to use the full name for unary operations.

@chokobole chokobole force-pushed the refac/rename-group-internal-operations branch 2 times, most recently from 1f0ee3c to c988a13 Compare April 18, 2024 08:29
@chokobole
refac: rename Negative() to Negate()
597cba2 
We decided to use the full name for the unary operations.
@chokobole
refac(node): rename negative() to negate()
e99dead 
We decided to use the full name for the unary operations.
@chokobole
refac: rename NegInPlace() to NegateInPlace()
70f1dce 
We decided to use the full name for the unary operations.
@chokobole
refac: rename DoSquareXXX to SquareImplXXX
e19ecf8 
This keeps consistency with other operations that have `DoXXX` as
a private method.
@chokobole
refac: rename DoDoubleXXX to DoubleImplXXX
fe78441 
This keeps consistency with other operations that have `DoXXX` as
a private method.
@chokobole chokobole force-pushed the refac/rename-group-internal-operations branch from c988a13 to fe78441 Compare April 18, 2024 08:30
ashjeong
ashjeong approved these changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@ashjeong ashjeong left a comment

Choose a reason for hiding this comment

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

LGTM

@chokobole chokobole merged commit 2d9e8c9 into main Apr 18, 2024
3 checks passed
@chokobole chokobole deleted the refac/rename-group-internal-operations branch April 18, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dongchangYoo dongchangYoo dongchangYoo approved these changes

@Insun35 Insun35 Insun35 approved these changes

@TomTaehoonKim TomTaehoonKim TomTaehoonKim approved these changes

@ashjeong ashjeong ashjeong 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.

None yet
5 participants
@chokobole @ashjeong @dongchangYoo @Insun35 @TomTaehoonKim