-
Notifications
You must be signed in to change notification settings - Fork 182
[#2429] Allow different time period for different repos in report-config.yaml #2490
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
base: master
Are you sure you want to change the base?
[#2429] Allow different time period for different repos in report-config.yaml #2490
Conversation
This help parsing different date time inputs formats.
New log message is added in ReportBranchData to show invalid date format entered by user. Branch
Also add checking of since date before until date in OneStopConfig
…ConfigYaml' into branch-diffirentTimePeriodReportConfigYaml
Accidentally commit changes to report-config.yaml when working with new report-config feature.
Accidentally commit changes to report-config.yaml when working with new report-config feature.
Accidentally commit changes to report-config.yaml when working with new report-config feature.
|
Sorry for the late reply.
It will use the config date provided and fall back to the cli date for the other that isn't specified in the config file. |
|
Hi, |
|
Hi, |
|
This PR was closed because it has been marked as stale for 7 days with no activity. |
lyuanww
left a comment
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.
@lihongong Sorry for the late review. The PR looks good overall—just a few small nits around documentation and tests before I can approve. Good work!
| * `since`: Start date of commits to include in the analysis. Accepted formats: | ||
| * Date only | ||
| * `dd/MM/yyyy` | ||
| * Date and time | ||
| * `dd/MM/yyyy HH:mm` or | ||
| * `dd/MM/yyyy HH:mm:ss` | ||
|
|
||
| * `until`: End date of commits to include in the analysis. Accepted formats: | ||
| * **same as the above** |
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.
| * `since`: Start date of commits to include in the analysis. Accepted formats: | |
| * Date only | |
| * `dd/MM/yyyy` | |
| * Date and time | |
| * `dd/MM/yyyy HH:mm` or | |
| * `dd/MM/yyyy HH:mm:ss` | |
| * `until`: End date of commits to include in the analysis. Accepted formats: | |
| * **same as the above** | |
| * `since`: Start date of commits to include in the analysis. Accepted formats: | |
| * Date format: | |
| * `dd/MM/yyyy` | |
| * Date and time formats: | |
| * `dd/MM/yyyy HH:mm` | |
| * `dd/MM/yyyy HH:mm:ss` (e.g., `05/12/2025 14:32:10`) | |
| * `until`: End date of commits to include in the analysis. Accepted formats: | |
| * Date format: | |
| * `dd/MM/yyyy` | |
| * Date and time formats: | |
| * `dd/MM/yyyy HH:mm` | |
| * `dd/MM/yyyy HH:mm:ss` (e.g., `05/12/2025 14:32:10`) |
Perhaps this is clearer?
| Behavior When Both Report Config Dates & CLI Flags Are Provided: | ||
|
|
||
| - If `since` date is specified in both `report-config.yaml` and CLI, the **Config dates take precedence** for that repository. The same applies to `until` date. | ||
| - The Config date range must fall within the **CLI date boundaries**. If it exceeds, the program exits with an error. | ||
| - Example: | ||
| - CLI Date - Since: 10/10/2024, Until: 20/10/2024 | ||
| - Valid Config Date - Since: 11/10/2024, Until: 19/10/2024 | ||
| - Invalid Config Date - Since: 9/10/2024, Until: 21/10/2024 | ||
| - If only one of the CLI flags is provided, the Config dates will not be restricted by the missing CLI flag. | ||
| - If Config dates are missing, the CLI flags are used. | ||
| - If neither Config nor CLI provides dates, default date ranges are used. |
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.
| Behavior When Both Report Config Dates & CLI Flags Are Provided: | |
| - If `since` date is specified in both `report-config.yaml` and CLI, the **Config dates take precedence** for that repository. The same applies to `until` date. | |
| - The Config date range must fall within the **CLI date boundaries**. If it exceeds, the program exits with an error. | |
| - Example: | |
| - CLI Date - Since: 10/10/2024, Until: 20/10/2024 | |
| - Valid Config Date - Since: 11/10/2024, Until: 19/10/2024 | |
| - Invalid Config Date - Since: 9/10/2024, Until: 21/10/2024 | |
| - If only one of the CLI flags is provided, the Config dates will not be restricted by the missing CLI flag. | |
| - If Config dates are missing, the CLI flags are used. | |
| - If neither Config nor CLI provides dates, default date ranges are used. | |
| **Behavior of `since`/`until` in report-config.yaml vs. `--since`/`--until` in CLI** | |
| * If `since` date is specified in both `report-config.yaml` and CLI, the config date takes priority for that repository. The same rule applies to the `until` date. | |
| * Config date range must fall **within** the CLI date range; otherwise the program exits with an error. | |
| * Example with CLI `--since` = 10/10/2024, `--until` = 20/10/2024: | |
| * Valid config dates: `since` = 11/10/2024, `until` = 19/10/2024 | |
| * Invalid config dates: `since` = 9/10/2024, `until` = 21/10/2024 | |
| * If only one CLI flag is provided, the missing CLI value does not restrict config dates. | |
| * If config dates are missing, CLI values are used. | |
| * If neither provides dates, default ranges are applied. |
Perhaps this is clearer?
| private static final Logger logger = LogsManager.getLogger(OneStopConfigRunConfiguration.class); | ||
| private static final String MESSAGE_CLI_CONFIG_DATE_CONFLICT = | ||
| "You specified in CLI a date range of --since to --until, " | ||
| + "but your report config specifies a date range that extends outside --SINCE or --UNTIL. " |
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.
| + "but your report config specifies a date range that extends outside --SINCE or --UNTIL. " | |
| + "but your report config specifies a date range that extends outside --since or --until. " |
|
|
||
| ReportBranchData branch = new ReportBranchData("master", "My project", authorList, | ||
| ignoreGlobList, ignoreAuthorList, 2000000L); | ||
| ignoreGlobList, ignoreAuthorList, 2000000L, sinceDate, untilDate); |
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.
Could you also add the values for expectedRepoConfigurations?
| String untilDateStr = "30/5/2025"; | ||
|
|
||
| ReportBranchData data = new ReportBranchData(branch, blurb, authors, ignoreGlobs, ignoreAuthors, fileSize); | ||
| ReportBranchData data = new ReportBranchData(branch, blurb, authors, ignoreGlobs, ignoreAuthors, fileSize, |
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.
Maybe you should add the assert for the new fields as well?
| // if both since and until date are provided in cli, then we need to check whether config date lays between | ||
| // the time period | ||
| if (configDate.isBefore(cliSinceDate) || configDate.isAfter(cliUntilDate)) { | ||
| throw new InvalidDatesException(MESSAGE_CLI_CONFIG_DATE_CONFLICT); |
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 you help to edit the comment description of InvalidDatesException.java?
Fixes #2429
Proposed commit message
report-config.yaml input:

Portfolio Mode:

Normal Mode:

