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

low hanging fruit #91

Merged
merged 17 commits into from
Dec 19, 2023
Merged

low hanging fruit #91

merged 17 commits into from
Dec 19, 2023

Conversation

fletch3555
Copy link
Contributor

@fletch3555 fletch3555 commented Dec 7, 2023

Some new examples:

  • digital-communication
  • dutycycle-encoder
  • dutycycle-input
  • encoder
  • flywheel-bangbang-controller
  • gyro-mecanum
  • hid-rumble
  • quick-vision
  • ramsete-controller

Also reformated/updated a few existing ones to be more consistent with java examples

@fletch3555 fletch3555 force-pushed the low-hanging-fruit branch 2 times, most recently from 0f036d5 to 0d0f64d Compare December 7, 2023 06:20
@fletch3555
Copy link
Contributor Author

It's becoming more apparent as I look through more and more of these examples, that there's a few different styles at play. I've been creating class-level constants with all-caps snakecase, but I see others following the k-prefix camelcase style. There are also a few others that define them in robotInit() instead of the class itself.

Is there a specific style you'd prefer to follow? I know the k-prefix is commonly used elsewhere in wpilib, but I've found the all-caps style to be more common in my professional experience. That said, I don't hold any strong opinions here. I'm happy to go update the ones I've already done.

@virtuald
Copy link
Member

virtuald commented Dec 7, 2023

Generally we follow the Java wpilib conventions, which uses k-prefix style. I don't care that much about which it is.

motor-control/robot.py Outdated Show resolved Hide resolved
@fletch3555
Copy link
Contributor Author

I'll stop adding to this so it can be properly reviewed. All the checks are green, so should be good to go now.

Copy link
Member

@virtuald virtuald left a comment

Choose a reason for hiding this comment

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

Looks like there are conflicts because of your file header changes.

flywheel-bangbang-controller/robot.py Outdated Show resolved Hide resolved
flywheel-bangbang-controller/robot.py Outdated Show resolved Hide resolved
ramsete-controller/robot.py Outdated Show resolved Hide resolved
ramsete-controller/robot.py Show resolved Hide resolved
@fletch3555 fletch3555 force-pushed the low-hanging-fruit branch 2 times, most recently from 6f7a944 to 61f2f7c Compare December 18, 2023 05:01
@virtuald virtuald merged commit 3c6757b into robotpy:main Dec 19, 2023
10 of 11 checks passed
@fletch3555 fletch3555 deleted the low-hanging-fruit branch December 20, 2023 04:36
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.

3 participants