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

Code cleanup for future FtcRobot controller #383

Open
thoughtless42 opened this issue Sep 27, 2022 · 7 comments
Open

Code cleanup for future FtcRobot controller #383

thoughtless42 opened this issue Sep 27, 2022 · 7 comments

Comments

@thoughtless42
Copy link

We have a script that takes care of these issues, but thought the larger ftc community could benefit from this request.
1.)FIRST-Tech-Challenge/FtcRobotController/FtcRobotController/build.gradle
compileOptions {
sourceCompatibility JavaVersion.VERSION_1_7
targetCompatibility JavaVersion.VERSION_1_7
}

should these be JavaVersion.VERSION_1_8 to match build.common.gradle ?

2.) FtcRobotController/FtcRobotController/src/main/assets/
this folder should be removed, it adds undue size to the robot controller apk and does not add value.
These historic files can be added to a new repository along with the pdf target files; split into separate dependencies like the last 2 seasons within build.dependencies.gradle
dependencies {
implementation 'org.firstinspires.ftc:gameAssets-PowerPlay:1.0.0'
}

3.) FtcRobotController/FtcRobotController/src/main/java/org/firstinspires/ftc/robotcontroller/internal/FtcOpModeRegister.java
remove import org.firstinspires.ftc.robotcontroller.external.samples.ConceptNullOp;
this import is not used and puts an undefined dependency on the external/samplesfolder on FtcOpModeRegister.java

@cmacfarl
Copy link
Member

Thank you for reporting this. It is being tracked internally.

@cmacfarl
Copy link
Member

FtcRobotController/FtcRobotController/src/main/java/org/firstinspires/ftc/robotcontroller/internal/FtcOpModeRegister.java
remove import org.firstinspires.ftc.robotcontroller.external.samples.ConceptNullOp;
this import is not used and puts an undefined dependency on the external/samplesfolder on FtcOpModeRegister.java

ConceptNullOp is actually being used in the javadoc with the @link property, which means it is required for the javadoc build so the resultant documentation can reference the class.

@thoughtless42
Copy link
Author

thoughtless42 commented Jun 12, 2023 via email

@Ely31
Copy link

Ely31 commented Sep 7, 2023

1.)FIRST-Tech-Challenge/FtcRobotController/FtcRobotController/build.gradle compileOptions { sourceCompatibility JavaVersion.VERSION_1_7 targetCompatibility JavaVersion.VERSION_1_7 }

should these be JavaVersion.VERSION_1_8 to match build.common.gradle ?

This was done in version 8.2.

@Ely31
Copy link

Ely31 commented Sep 11, 2023

the assets folder was removed in 9.0

qwertychouskie added a commit to qwertychouskie/FtcRobotController that referenced this issue Oct 30, 2023
Yes, this is intended to be an actual PR, and is not a mistake.

See FIRST-Tech-Challenge#383 (comment)

Closes FIRST-Tech-Challenge#383 (as all the other listed issues have been fixed.)
@qwertychouskie
Copy link

if line 52 was * See, for example the class annotations in {@link org.firstinspires.ftc.robotcontroller.external.samples.ConceptNullOp}. then the import on line 37 import org.firstinspires.ftc.robotcontroller.external.samples.ConceptNullOp; should be able to be removed without breaking javadoc

Created #802 but it was auto-closed.

@qwertychouskie
Copy link

@cmacfarl Just wanted to check in on the status of fixing the last part of this issue. The fix already exists (#802) but due to the nature of SDK development, there's seemingly no way to properly submit these changes to upstream. I also came across some more simple-but-potent optimizations in #797 but once again sadly PRs are not possible.

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 a pull request may close this issue.

4 participants