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

[wpimath] change G to gearing in LinearSystemId factories #5834

Merged
merged 9 commits into from
Nov 13, 2023

Conversation

DeltaDizzy
Copy link
Contributor

The gearing parameter being called "G" was often confusing when using LinearSystemId, and making the name more descriptive would help avoid that.

@DeltaDizzy DeltaDizzy requested a review from a team as a code owner October 27, 2023 01:59
Copy link
Member

@auscompgeek auscompgeek 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 the C++ version needs to be updated too.

@DeltaDizzy
Copy link
Contributor Author

I think the C++ version needs to be updated too.

I saw a suggestion from rzblue in the discord about this and already changed it.

@DeltaDizzy
Copy link
Contributor Author

/format

@rzblue
Copy link
Member

rzblue commented Oct 27, 2023

You'll need to update the header as well

@DeltaDizzy
Copy link
Contributor Author

/format

@DeltaDizzy DeltaDizzy changed the title Change G to gearing in LinearSystemId factories [wpimath] change G to gearing in LinearSystemId factories Oct 27, 2023
@KangarooKoala
Copy link
Contributor

Java still capitalizes "gearing" in the exception message.

@PeterJohnson PeterJohnson merged commit 78ebc6e into wpilibsuite:main Nov 13, 2023
24 checks passed
@DeltaDizzy DeltaDizzy deleted the systemid-gearingparam branch November 13, 2023 04:57
Starlight220 pushed a commit to Starlight220/allwpilib that referenced this pull request Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants