-
Notifications
You must be signed in to change notification settings - Fork 164
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
[#2161] One-Stop Config File for Code Portfolio #2192
base: master
Are you sure you want to change the base?
[#2161] One-Stop Config File for Code Portfolio #2192
Conversation
…implement-one-stop-config-file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on the YAML config file @asdfghjkxd! I have some questions and left them as comments below.
@@ -107,12 +107,22 @@ e.g.: `example.java` in `example-repo` can either be in the `test` group or the | |||
|
|||
<!-- ==================================================================================================== --> | |||
|
|||
## `report-config.json` | |||
## `report-config.yaml` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also update the --config
flag under 'CLI syntax reference' to include report-config.yaml
instead of json
? https://docs-2192-pr-reposense-reposense.surge.sh/ug/cli.html#config-c
Let's also update the 'Customize using CSV config files' heading on the 'Customizing Reports` page to include YAML or remove file types entirely. https://docs-2192-pr-reposense-reposense.surge.sh/ug/customizingReports.html#customize-using-csv-config-files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do!
public static final List<String> DEFAULT_FILE_FORMATS = List.of( | ||
"override:java", "md", "fxml" | ||
); | ||
public static final List<String> DEFAULT_IGNORE_GLOB_LIST = List.of( | ||
"docs**" | ||
); | ||
public static final List<String> DEFAULT_IGNORE_COMMITS_LIST = List.of( | ||
"2fb6b9b2dd9fa40bf0f9815da2cb0ae8731436c7", | ||
"c5a6dc774e22099cd9ddeb0faff1e75f9cf4f151", | ||
"cd7f610e0becbdf331d5231887d8010a689f87c7", | ||
"768015345e70f06add2a8b7d1f901dc07bf70582" | ||
); | ||
public static final List<String> DEFAULT_IGNORE_AUTHORS_LIST = List.of( | ||
"author1", | ||
"author2" | ||
); | ||
public static final boolean DEFAULT_IS_FIND_PREVIOUS_AUTHOR = false; | ||
public static final boolean DEFAULT_IS_SHALLOW_CLONING = true; | ||
public static final boolean DEFAULT_IS_IGNORE_STANDALONE_CONFIG = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we need these hardcoded default values here? Check for this in the other files inside reportconfig/
too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe they are present for testing purposes (creating default instances), but I will have to check before reverting back to you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they're just for tests, could we move them into the test files like we do for test defaults for the other classes? It doesn't seem like these values are useful defaults outside of the tests.
Hi, |
This PR was closed because it has been marked as stale for 7 days with no activity. |
If anyone would like to jump onto this to complete the feature, please feel free to do so! |
reposense#2260) Update GitHub Action runner to macOS 13, 14, 15 and remove macOS 12 The macOS 12 version is deprecated and removed by the end of 2024. This commit updates the GitHub Action runner to use macOS 13, 14, and 15 instead.
Add ubuntu-24.04 to CI runners This PR updates the CI configuration to include Ubuntu 24.04 as one of the runners. This ensures that our code is tested on the latest Ubuntu version, providing better compatibility and early detection of potential issues.
…sense#2274) Upgrade artifact actions to v4 All workflows using v3 of artifact actions automatically fail from 2025-01-30 onwards due to deprecation. Let's upgrade to the latest version (v4).
Update apt package
Update codecov version The depreciation of older version of codecov script causes token-less upload of coverage report fail. Let's update the codecov action to the latest version. --------- Co-authored-by: CHENYIXUN <guest1@CHENYIXUN>
…ackend Task 1 (reposense#2268) * Update suggested solution for backend task 1 hint 3 * Remove error * Update suggested solution for backend task 1 hint 2 --------- Co-authored-by: Ryan Poon <[email protected]>
Set UTC timezone for test reports Running Cypress tests locally uses system's default timezone (UTC+8), causing 3 tests that check for dates to fail. These 3 tests pass on GitHub Actions as the system's default timezone is UTC. Let's standardize the timezone for all cypress tests to UTC.
…eposense#2270) Update GSON type adapter adding methods The registration of custom adapters for ZoneID and LocalDateTime causes The JsonIOException to pop up when trying to run the jar file in java 17. Let's use the registerTypeHierarchyAdapter method to add the custom adapters instead.
Fix the <hr> element not appearing in report The <hr> element in title.md and blurbs.md is not appearing in the report because the normalise.css defaults the <hr> element style. Let's override the <hr> element style in style.scss so that it will appear in the report.
Move from Vue CLI to Vite Vue CLI has been put on maintenance mode and its dependencies have security issues that have not been fixed for a while. Switching to Vite also allows us to introduce Vitest for our unit testing. Let's install Vite and remove Vue CLI.
…eorgetayqy/RepoSense into implement-one-stop-config-file
Part of #2161
Proposed commit message
Other information
Requires CI/CD to be updated to Java 11: