Programming exercises: Add R programming language template
#9256
Conversation
github-actions
bot
added
server
magaupp
changed the title
Programming exercises: Add R programming language template
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
magaupp
force-pushed
the
feature/programming-exercises/r-template
branch
from
3fdade7
to
6b618c6
Compare
magaupp
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
GitHub Actions
Inactive
magaupp
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
GitHub Actions
Inactive
WalkthroughThis pull request introduces support for the R programming language across various components of the application. It updates the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Files ignored due to path filters (2)
src/main/resources/config/application.ymlis excluded by!**/*.ymlsrc/main/resources/templates/aeolus/r/default.yamlis excluded by!**/*.yaml
Files selected for processing (23)
- docs/user/exercises/programming-exercise-features.inc (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/TemplateUpgradePolicyService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/ci/ContinuousIntegrationService.java (3 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/build_plan/JenkinsBuildPlanService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (2 hunks)
- src/main/resources/templates/aeolus/r/default.sh (1 hunks)
- src/main/resources/templates/gitlabci/r/regularRuns/.gitlab-ci.yml (1 hunks)
- src/main/resources/templates/jenkins/r/regularRuns/pipeline.groovy (1 hunks)
- src/main/resources/templates/r/exercise/DESCRIPTION (1 hunks)
- src/main/resources/templates/r/exercise/NAMESPACE (1 hunks)
- src/main/resources/templates/r/exercise/R/convert.R (1 hunks)
- src/main/resources/templates/r/readme (1 hunks)
- src/main/resources/templates/r/solution/DESCRIPTION (1 hunks)
- src/main/resources/templates/r/solution/NAMESPACE (1 hunks)
- src/main/resources/templates/r/solution/R/convert.R (1 hunks)
- src/main/resources/templates/r/test/DESCRIPTION (1 hunks)
- src/main/resources/templates/r/test/tests/testthat.R (1 hunks)
- src/main/resources/templates/r/test/tests/testthat/test-convert.R (1 hunks)
- src/main/webapp/app/entities/programming/programming-exercise.model.ts (1 hunks)
- src/main/webapp/app/exercises/programming/shared/code-editor/file-browser/supported-file-extensions.ts (1 hunks)
Additional context used
Path-based instructions (9)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/TemplateUpgradePolicyService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/webapp/app/exercises/programming/shared/code-editor/file-browser/supported-file-extensions.ts (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/webapp/app/entities/programming/programming-exercise.model.ts (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/ci/ContinuousIntegrationService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/build_plan/JenkinsBuildPlanService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
Learnings (5)
src/main/resources/templates/aeolus/r/default.sh (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/resources/templates/aeolus/rust/default.sh:14-24 Timestamp: 2024-07-01T10:48:58.558Z Learning: The `default.sh` script in the `src/main/resources/templates/aeolus/rust/` directory is auto-generated by Aeolus and should not be manually edited. Changes need to be made in the Aeolus generator settings or templates.src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/gitlabci/GitLabCIProgrammingLanguageFeatureService.java:24-24 Timestamp: 2024-06-29T14:49:34.001Z Learning: Static code analysis for Rust in GitLab CI will be added in a follow-up PR.src/main/java/de/tum/cit/aet/artemis/programming/service/TemplateUpgradePolicyService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#9074 File: src/main/java/de/tum/in/www1/artemis/service/programming/TemplateUpgradePolicyService.java:35-35 Timestamp: 2024-07-16T20:00:50.653Z Learning: The `defaultRepositoryUpgradeService` in the `TemplateUpgradePolicyService` is used as a default for all non-JAVA languages, with specialized services added based on testing outcomes if necessary.src/main/resources/templates/gitlabci/r/regularRuns/.gitlab-ci.yml (1)
Learnt from: magaupp PR: ls1intum/Artemis#8859 File: src/main/resources/templates/gitlabci/javascript/regularRuns/.gitlab-ci.yml:36-49 Timestamp: 2024-07-16T22:00:10.722Z Learning: In the Artemis project, the `gradle run` command in the JavaScript CI configuration is used to operate a Java-based notification plugin.src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/jenkins/JenkinsProgrammingLanguageFeatureService.java:43-43 Timestamp: 2024-06-29T14:47:20.796Z Learning: Jenkins support for Rust was added in commit d0fb7032e70c001dc8b2c4d0a391ca8bf7e7d586.
Additional comments not posted (41)
src/main/resources/templates/r/exercise/NAMESPACE (1)
1-1: LGTM! The export pattern is well-defined and serves an important purpose.The
exportPattern("^[^\\.]")line effectively controls the visibility of module components by exporting only entities whose names do not start with a dot (.). This pattern is crucial for:
- Enhancing encapsulation and promoting better organization of code.
- Filtering out private or internal entities that typically begin with a dot.
- Allowing developers to manage dependencies and visibility more effectively.
By implementing this export pattern, the module ensures that only appropriately named entities are accessible to other modules or components that import this namespace.
src/main/resources/templates/r/solution/NAMESPACE (1)
1-1: LGTM! The exported pattern enhances modularity and maintainability.The
exportPattern("^[^\\.]")line effectively controls the visibility of elements within the module or namespace. By ensuring that only elements not starting with a dot (.) are exported, it encapsulates internal logic while exposing only the necessary components to other parts of the application.This pattern provides several benefits:
- It prevents unintended access to internal elements that are not meant to be public.
- It enhances modularity by clearly defining the public interface of the module.
- It improves maintainability by reducing the risk of external code relying on internal implementation details.
Great job in implementing this pattern to manage the scope of variables, functions, or classes in the R solution template!
src/main/resources/templates/r/exercise/DESCRIPTION (7)
1-1: LGTM!The package name "assignment" is appropriate for a student assignment and follows R package naming conventions.
2-2: LGTM!The package title clearly indicates that this is an R assignment for the Artemis platform.
3-3: LGTM!The version number 0.0.0.9000 follows semantic versioning and indicates an initial development version, which is appropriate for a new package.
4-4: LGTM!Listing "Artemis" as the author is suitable for an assignment created by the Artemis platform.
5-5: LGTM!The description concisely states the purpose of the package as a student assignment, which provides clarity on its educational purpose.
6-6: LGTM!The MIT license is a permissive open-source license, which is a good choice for an educational assignment as it allows for broad usage and distribution.
7-7: LGTM!The UTF-8 encoding ensures compatibility with a wide range of characters, which is important for internationalization and proper display of text.
src/main/resources/templates/r/solution/DESCRIPTION (1)
1-7: LGTM!The
DESCRIPTIONfile follows the standard format for an R package and includes all the necessary metadata. The chosen license (MIT) and encoding (UTF-8) are appropriate for the package's educational purpose and ensure broad compatibility.This file is essential for the proper functioning and distribution of the R package within the Artemis platform.
src/main/resources/templates/r/test/DESCRIPTION (1)
1-14: LGTM!The
DESCRIPTIONfile for thetestpackage is well-structured and follows the standard format for R packages. The metadata, dependencies, and configuration are clearly defined and appropriate for the package's purpose of testing student assignments within the Artemis framework.The dependencies on the
assignmentpackage and thetestthattesting framework are correctly specified, ensuring that the package can effectively test student assignments and leverage standard testing practices.Overall, the file sets up a solid foundation for the
testpackage to function effectively within the Artemis framework and the R ecosystem.src/main/resources/templates/r/readme (1)
1-6: LGTM!The readme provides a clear description of the
matrix_to_column_listfunction and outlines a comprehensive task to test the function with matrices of different shapes. This will help ensure the robustness of the function implementation.src/main/resources/templates/r/test/tests/testthat.R (1)
1-12: LGTM!The
testthat.Rfile follows the standard setup for thetestthattesting framework in R packages. It correctly loads the necessary libraries and includes clear comments explaining the purpose and usage of the file. The code adheres to best practices for organizing tests in an R package.src/main/resources/templates/aeolus/r/default.sh (4)
4-7: LGTM!The
installfunction correctly uses theR CMD INSTALLcommand to install the R package.
9-12: LGTM!The
run_all_testsfunction correctly uses theRscriptcommand to run tests using thetestthatlibrary and generate a JUnit report.
14-24: LGTM!The
mainfunction correctly handles the "aeolus_sourcing" argument, changes the directory back to the initial directory, and calls theinstallandrun_all_testsfunctions.
1-26: Reminder: Do not manually edit this file.The
default.shscript is auto-generated by Aeolus and should not be manually edited. Changes need to be made in the Aeolus generator settings or templates.src/main/resources/templates/r/test/tests/testthat/test-convert.R (4)
1-12: LGTM!The test case is well-structured and follows the best practice of using only one
expect_function per test. It correctly tests the conversion of a 3x3 matrix into a list of three vectors.
14-23: LGTM!The test case is well-structured and follows the best practice of using only one
expect_function per test. It correctly tests the conversion of a 4x2 matrix into a list of two vectors.
25-37: LGTM!The test case is well-structured and follows the best practice of using only one
expect_function per test. It correctly tests the conversion of a 1x5 matrix into a list of five scalars.
39-47: LGTM!The test case is well-structured and follows the best practice of using only one
expect_function per test. It correctly tests the conversion of a 5x1 matrix into a list containing a single vector.src/main/resources/templates/jenkins/r/regularRuns/pipeline.groovy (1)
1-59: LGTM!The Jenkins pipeline script for automatic grading of R assignments is well-structured and follows best practices. Here are some key observations:
- The script uses Docker to ensure a consistent and isolated environment for running tests.
- It follows a modular structure with clear separation of concerns, making it easy to maintain and extend.
- The use of JUnit XML format for reporting test results is a good choice as it integrates well with Jenkins and other CI/CD tools.
- The
postBuildTasksfunction ensures that the test results are properly aggregated and cleaned up.- The script includes appropriate documentation and comments explaining its purpose and usage.
Overall, the script provides a robust and standardized way to automate the grading process for R assignments. Great job!
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (2)
5-5: LGTM!The import statement for the
Renum value is necessary and follows the existing import style. It is placed in the correct location along with other imports from theProgrammingLanguageenum.
29-29: LGTM!The code segment adds a new entry to the
programmingLanguageFeaturesmap for the R programming language, following the existing pattern for other languages. TheProgrammingLanguageFeatureinstance is initialized with sensible default values, consistent with the entries for other languages.This change enhances the service's capability to handle R as a recognized programming language and allows for future extensions or configurations related to R in the context of GitLab CI features. It does not introduce any breaking changes or impact existing functionality.
src/main/java/de/tum/cit/aet/artemis/programming/service/TemplateUpgradePolicyService.java (2)
35-35: LGTM!The change to include the R programming language in the case statement that returns the
defaultRepositoryUpgradeServiceis consistent with the learning thatdefaultRepositoryUpgradeServiceis used as a default for all non-JAVA languages. This change enables support for the R programming language without impacting the existing functionality.
36-36: LGTM!The change to remove the R programming language from the list of unsupported languages that throw an
UnsupportedOperationExceptionis consistent with the previous change that added support for the R programming language. This ensures that the R programming language is no longer treated as an unsupported language, without impacting the existing functionality.src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1)
51-52: LGTM!The addition of the
Rconstant to theENABLED_LANGUAGESset is consistent with the existing pattern and enables the R programming language in the application, aligning with the PR objective.src/main/resources/templates/gitlabci/r/regularRuns/.gitlab-ci.yml (2)
6-33: LGTM!The
test-jobis well-structured and follows a logical sequence of steps. It uses appropriate commands and tools for cloning repositories, installing packages, and running tests. The modification of the JUnit report format is a necessary workaround for compatibility with the notification plugin. The job captures important information such as build status, commit hashes, and artifacts, which facilitates the integration of R projects into the CI/CD pipeline.
35-48: Verify the usage of the Gradle command.The
upload-jobuses a Gradle command to run the upload process, which suggests that the notification plugin is implemented in Java. This is similar to the JavaScript CI configuration where thegradle runcommand is used to operate a Java-based notification plugin, as mentioned in the provided learning.To confirm this, please run the following script to search for the usage of the
gradle runcommand in the JavaScript CI configuration:Verification successful
Consistent usage of 'gradle run' command across language configurations
The 'gradle run' command is consistently used in multiple language-specific CI configurations, including R, Java (Maven), Rust, and an "empty" configuration. This suggests that the command is part of a language-agnostic process, likely related to the notification plugin as initially suspected.
- The command is present in the R configuration file:
src/main/resources/templates/gitlabci/r/regularRuns/.gitlab-ci.yml- It's also used in Java (Maven), Rust, and a generic "empty" configuration
This consistency across different language setups supports the original implementation of the upload process using a Java-based notification plugin. The absence of a JavaScript-specific configuration file indicates that our initial assumption about a separate JavaScript CI configuration was incorrect.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `gradle run` command in the JavaScript CI configuration. # Test: Search for the `gradle run` command in the JavaScript CI configuration file. # Expect: The command is used to operate a Java-based notification plugin. rg --type yaml --ignore-case $'gradle run' src/main/resources/templates/gitlabci/javascript/regularRuns/.gitlab-ci.ymlLength of output: 235
Script:
#!/bin/bash # Description: Locate the JavaScript CI configuration file and search for the `gradle run` command. # Test 1: Find all .gitlab-ci.yml files in the src directory that might be related to JavaScript echo "Searching for JavaScript .gitlab-ci.yml files:" fd -p 'src/.*javascript.*\.gitlab-ci\.yml$' # Test 2: Search for 'gradle run' in all YAML files within the src directory echo -e "\nSearching for 'gradle run' in all YAML files:" rg --type yaml --ignore-case 'gradle run' srcLength of output: 677
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (2)
9-9: LGTM!The import statement for the
Rprogramming language is consistent with the existing code style and pattern. It is necessary to use theRprogramming language in this class.
45-45: Looks good!The addition of the
Rprogramming language to theprogrammingLanguageFeaturesmap is consistent with the existing code style and pattern. It expands the service's capability to handle this programming language and enhances the extensibility of the service.Setting all feature flags to
falseindicates that theRprogramming language does not support any specific features at this time, which is a reasonable default.src/main/webapp/app/exercises/programming/shared/code-editor/file-browser/supported-file-extensions.ts (1)
3-3: LGTM!The addition of 'R' to the
supportedTextFileExtensionsarray is a small but important part of the larger effort to introduce support for the R programming language across the application. The change is consistent with the purpose of the array and maintains its alphabetical order.src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (2)
11-11: LGTM!The import statement for the
Rprogramming language enum is added correctly, following the existing conventions.
52-52: Looks good!The new entry for the
Rprogramming language is added correctly to theprogrammingLanguageFeaturesmap. TheProgrammingLanguageFeatureinstance is initialized with the appropriate parameters forR.This change enhances the service's capability to handle programming language features for
R, expanding its functionality.src/main/webapp/app/entities/programming/programming-exercise.model.ts (1)
27-27: LGTM! The addition of the R programming language aligns with the PR objectives.The new enum value
R = 'R'is added correctly to theProgrammingLanguageenum, following the existing naming convention. This change enhances the application's functionality by allowing it to recognize and potentially support the R programming language in contexts where programming languages are utilized, such as programming exercises or assessments.Please ensure that corresponding updates are made in other relevant parts of the codebase to fully integrate R programming exercises, such as:
- Updating documentation and user interfaces to include R as a supported language option.
- Implementing necessary backend services and configurations to handle R-based exercises.
- Adding appropriate test cases to verify the functionality of R programming exercises.
src/main/java/de/tum/cit/aet/artemis/programming/service/ci/ContinuousIntegrationService.java (3)
222-223: LGTM!The addition of the
Rprogramming language case is consistent with the existing behavior for other supported languages. It indicates thatRis now supported for assignments and the assignment repository should be checked out to the "assignment" subdirectory.
233-235: Looks good!The addition of the
Rprogramming language case aligns with the existing logic for test repositories. Returning an empty string forRindicates that the test repository should be checked out to the root directory, which is consistent with the behavior for other languages likeJAVA,PYTHON, etc.
245-247: Looks good to me!The addition of the
Rprogramming language case follows the pattern for solution repositories. Returning "solution" forRindicates that the solution repository should be checked out to the "solution" subdirectory, which aligns with the behavior for other languages likeHASKELLandOCAML.docs/user/exercises/programming-exercise-features.inc (2)
38-39: LGTM!The addition of R to the programming language support table is consistent with the PR objective. The table structure and formatting are maintained.
72-73: Looks good!The addition of the R programming language to the feature support table is consistent with the PR objective. The supported features for R are appropriately marked, similar to Rust. The table structure and formatting are maintained.
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/build_plan/JenkinsBuildPlanService.java (1)
187-188: LGTM!The code changes add support for the
Rprogramming language in thebuilderFormethod, which is consistent with the list of alterations provided in the summary. The implementation looks good.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range comments (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Line range hint
1-33: Consider the impact of adding features to a deprecated class.While the addition of R language support is implemented correctly, it's worth noting that this class is marked as deprecated and scheduled for removal in version 8.0.0. Adding new features to a deprecated class may lead to maintenance issues in the future.
Consider the following alternatives:
- If R support is crucial before the removal of GitLab CI, proceed with caution and ensure proper migration documentation.
- If possible, implement R support directly in the new system that will replace GitLab CI (mentioned as LocalVC in the comment).
- Create a separate, non-deprecated class for R support that can be easily migrated or integrated into the new system.
Please clarify the strategy for handling this addition in light of the planned deprecation.
build.gradle (1)
Line range hint
1-1010: Recommendation for testing and documentation.The addition of R language support appears to be correctly implemented in the
build.gradlefile. To ensure a smooth integration, please:
- Run a full build and test cycle to verify that the R language dependency doesn't introduce any conflicts or issues.
- Update the project's documentation, including the README and any developer guides, to mention the new R language support.
- If there are any specific setup or configuration steps required for R language support, add them to the project's setup instructions.
These steps will help maintain the project's stability and keep all developers informed about the new language support.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (6)
- build.gradle (1 hunks)
- docs/user/exercises/programming-exercise-features.inc (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
📓 Learnings (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/gitlabci/GitLabCIProgrammingLanguageFeatureService.java:24-24 Timestamp: 2024-06-29T14:49:34.001Z Learning: Static code analysis for Rust in GitLab CI will be added in a follow-up PR.src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/jenkins/JenkinsProgrammingLanguageFeatureService.java:43-43 Timestamp: 2024-06-29T14:47:20.796Z Learning: Jenkins support for Rust was added in commit d0fb7032e70c001dc8b2c4d0a391ca8bf7e7d586.
🔇 Additional comments (8)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
6-6: LGTM: Import statement for R language added correctly.The static import for the R programming language is consistent with other language imports and follows the coding guidelines.
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
10-10: LGTM: Import statement for R is correctly added.The import statement for R is properly placed with other similar imports and follows the coding guideline to avoid star imports.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
55-55: LGTM: R language support correctly added.The addition of R programming language support is implemented correctly and consistently with other language entries. It adheres to the coding guidelines, including proper naming conventions and principles. The structure matches the existing pattern, which promotes maintainability.
Regarding the past review suggestion about adding comments for readability, I acknowledge the author's response that most IDEs can show parameter names. If this is a standard practice in your team and doesn't hinder understanding for new team members, then the current implementation is acceptable.
docs/user/exercises/programming-exercise-features.inc (1)
40-41: LGTM: R language support added correctly.The addition of R to the programming language support table is consistent with the existing structure and aligns with the PR objectives.
src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (2)
40-40: Import statement for R language support added correctly.The import statement for
de.jplag.rlang.RLanguageis appropriately placed with other JPlag language imports and follows Java naming conventions.
40-40: R language support successfully integrated into the plagiarism detection service.The changes effectively introduce support for the R programming language in the
ProgrammingPlagiarismDetectionServiceclass. The import statement and the modification to thegetJPlagProgrammingLanguagemethod are consistent with the existing code structure and style. These changes align well with the PR objectives of integrating R language support into the Artemis platform.Also applies to: 321-322
build.gradle (2)
Line range hint
252-262: Verify complete R language integration in build process.The R language dependency has been added correctly among other JPlag language dependencies. To ensure full integration, please verify the following:
- Check if any task definitions or custom configurations in the build file need to be updated to include R language support.
- Verify that any language-specific plugins or tools used in the build process are compatible with R language files.
- If there are any test configurations, ensure they cover R language files as well.
These steps will help guarantee that R language support is fully integrated into all aspects of the build and test process.
To check for any R-specific configurations or tasks, you can run:
#!/bin/bash # Search for any existing R-related configurations or tasks grep -n -i "r.*lang" build.gradle
257-257: R language support added successfully.The addition of the R language dependency
implementation "de.jplag:rlang:${jplag_version}"is consistent with the other language dependencies in this section. This change successfully integrates R language support into the project.However, to ensure complete integration, consider the following:
- Verify that
jplag_versionis defined in the project'sgradle.propertiesfile or elsewhere in the build script.- Check if any additional configurations or plugins are needed to fully support R language processing in your project.
- Update any relevant documentation or README files to mention the new R language support.
To confirm the
jplag_versionis properly defined, you can run:✅ Verification successful
To accurately verify the definition and value of
jplag_version, please run the following shell script:
jplag_versionis defined correctly.The
jplag_versionis set to5.1.0ingradle.properties, ensuring that the R language dependency uses the specified version.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if jplag_version is defined in gradle.properties grep -q "jplag_version" gradle.properties && echo "jplag_version is defined" || echo "jplag_version is not defined" # Display the value of jplag_version if it exists grep "jplag_version" gradle.propertiesLength of output: 297
Script:
#!/bin/bash # Check if jplag_version is defined in gradle.properties if grep -q "^jplag_version=" gradle.properties; then echo "jplag_version is defined." # Display the value of jplag_version grep "^jplag_version=" gradle.properties else echo "jplag_version is not defined." fiLength of output: 169
...um/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java
Outdated
Show resolved
Hide resolved
...in/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java
Outdated
Show resolved
Hide resolved
This should reduce merge conflicts by reducing changes to adjacent lines
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range comments (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Line range hint
20-21: Consider the impact of adding features to a deprecated class.While the additions for R language support and updates to Rust are valid, it's worth noting that this class is marked as deprecated.
Given the deprecation notice and the recommendation to migrate to LocalVC, consider the following:
- Evaluate if these changes should also be implemented in the new LocalVC system.
- Ensure that the migration guide or documentation is updated to reflect these new language supports.
- Consider adding a comment explaining why new features are being added to a deprecated class, if this is a deliberate decision.
This will help maintain consistency and smooth the transition for users migrating from GitLab CI to LocalVC.
Also applies to: 30-31
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (7)
- build.gradle (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (2 hunks)
- src/main/webapp/app/entities/programming/programming-exercise.model.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/webapp/app/entities/programming/programming-exercise.model.ts (1)
📓 Learnings (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/gitlabci/GitLabCIProgrammingLanguageFeatureService.java:24-24 Timestamp: 2024-06-29T14:49:34.001Z Learning: Static code analysis for Rust in GitLab CI will be added in a follow-up PR.src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/java/de/tum/in/www1/artemis/service/connectors/jenkins/JenkinsProgrammingLanguageFeatureService.java:43-43 Timestamp: 2024-06-29T14:47:20.796Z Learning: Jenkins support for Rust was added in commit d0fb7032e70c001dc8b2c4d0a391ca8bf7e7d586.
🔇 Additional comments (14)
src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1)
Line range hint
15-15: LGTM: R language added correctlyThe addition of the R language to the
ProgrammingLanguageenum is correct and consistent with the existing structure. The file extension "R" is appropriate for R language files.src/main/java/de/tum/cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java (2)
6-6: LGTM: R language import added correctly.The import statement for the R language is correctly placed and follows the coding guideline of avoiding star imports.
31-31: LGTM: Rust language support updated. Clarification needed on static code analysis.The update to Rust language support is consistent with the R language entry. However, I noticed that the static code analysis flag is set to true for Rust.
Could you please clarify if this is intentional? There's a note mentioning that static code analysis for Rust will be added in a follow-up PR, but it appears to be already enabled here.
src/main/webapp/app/entities/programming/programming-exercise.model.ts (1)
16-28: 🧹 Nitpick (assertive)Consider sorting enum entries alphabetically for better maintainability.
The addition of 'R' to the
ProgrammingLanguageenum aligns with the PR objectives. However, the current ordering of entries doesn't follow a clear pattern, which may impact readability and maintainability.To improve this, consider sorting the enum entries alphabetically. This would make it easier to locate specific entries and maintain consistency as new languages are added in the future.
Here's a suggested alphabetical order:
export enum ProgrammingLanguage { ASSEMBLER = 'ASSEMBLER', C = 'C', EMPTY = 'EMPTY', HASKELL = 'HASKELL', JAVA = 'JAVA', JAVASCRIPT = 'JAVASCRIPT', KOTLIN = 'KOTLIN', OCAML = 'OCAML', PYTHON = 'PYTHON', R = 'R', RUST = 'RUST', SWIFT = 'SWIFT', VHDL = 'VHDL', }This change would enhance code readability while maintaining the intended functionality.
src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (2)
40-40: Import for R language support added correctly.The import statement for
de.jplag.rlang.RLanguageis correctly placed and consistent with other language imports. This addition is necessary for supporting R language in the plagiarism detection service.
Line range hint
1-524: R language support successfully integrated into the plagiarism detection service.The changes made to this file successfully integrate support for the R programming language in the plagiarism detection service. The modifications are consistent with the existing code structure and style, and no further changes appear necessary for R language support in this file.
These changes align well with the PR objectives of integrating R programming language into the Artemis platform, responding to requests from students at the University Innsbruck.
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (5)
10-10: Import statement for R added correctly.The import statement for
Rhas been added appropriately.
37-38: Verify the inclusion of C and HASKELL entries.The entries for
CandHASKELLare present in theprogrammingLanguageFeaturesmap. Please verify whether reintroducing these programming languages is intentional, as they appear to have been removed previously.
41-41: Verify the inclusion of JAVASCRIPT entry.The
JAVASCRIPTentry is included in theprogrammingLanguageFeaturesmap. Confirm if this addition is intended.
44-44: Addition of R programming language support is implemented correctly.The
Rprogramming language has been correctly added to theprogrammingLanguageFeaturesmap with appropriate feature flags.
45-45: Verify the inclusion of RUST entry.The
RUSTprogramming language entry is included. Please confirm if reintroducingRUSTis intentional.src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (3)
13-13: Correctly imported the R programming languageThe import statement for
ProgrammingLanguage.Ron line 13 is properly added, enabling the use of the R language in the service.
43-45: Verify the re-insertion of language entriesLines 43-45 reintroduce entries for
ASSEMBLER,C, andHASKELLinto theprogrammingLanguageFeaturesmap. Please confirm that this re-addition is intentional and that their configurations match the expected feature sets.
52-52: R programming language features configured correctlyLine 52 adds the R programming language to the
programmingLanguageFeaturesmap with the following settings:
hasFormatter:falsehasStaticCodeAnalysis:falsehasTestCoverage:truehasInlineCodeEditing:falsehasInlineErrorFeedback:falsesupportedProjectTypes:List.of()supportsDebugging:falsesupportsIncrementalBuild:trueThese configurations appear appropriate for the R language within the local CI environment.
src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java
Show resolved
Hide resolved
.../cit/aet/artemis/programming/service/gitlabci/GitLabCIProgrammingLanguageFeatureService.java
Outdated
Show resolved
Hide resolved
...in/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java
Show resolved
Hide resolved
...um/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java
Show resolved
Hide resolved
BBesrour
temporarily deployed
to
artemis-test6.artemis.cit.tum.de
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range comments (1)
build.gradle (1)
Line range hint
1-1010: Consider enhancing dependency management and organization.While the current build file is functional, there are a couple of suggestions that could improve maintainability and readability:
Consider using a dependency management plugin like "com.github.ben-manes.versions" more extensively. This can help in keeping dependencies up-to-date and managing versions more effectively.
Group similar dependencies together. For example, all Spring-related dependencies could be grouped, all testing dependencies could be in another group, etc. This can improve readability and make it easier to manage dependencies in the future.
Would you like me to provide an example of how to implement these suggestions?
magaupp
dismissed stale reviews from BBesrour, coderabbitai[bot], and JohannesStoehr
via
71f01fe
Checklist
General
Server
Changes affecting Programming Exercises
Motivation and Context
Students from the University Innsbruck requested support for integrating the R language into Artemis.
Description
This PR adds the R programming language as an exercise template. It is structured according to R Packages (2e) and uses
testthatas the testing framework. Thanks to Johanna and Marco for providing an exercise.Steps for Testing
Note that GitLab CI is not supported.
Prerequisites:
RlanguageTemplate Resultshould show 0 passed tests and no build failureSolution Resultshould show 4 passed tests and no build failureTestserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Test Coverage
Screenshots
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
Enhancements
Bug Fixes