RepoSense
RepoSense copied to clipboard
Enforce argument priority order
Step to reproduce:
Enter the command that contains -formats field (java in the example)
However, the dashboard contains unrelated format
The reason for this issue is that it will autoload the format in the config.json file. Do we need to set the isStandaloneConfigIgnored to true when using LocationsCliArguments? Or the level of format in the config.json file is meant to be higher than the one in command line & repo-config.csv ?
Can be joint into the discussion of #390
@fzdy1914 is this issue still relevant?
It is still valid. In https://github.com/reposense/RepoSense/issues/390, we have allowed the config in repo-config.csv
to override values in standalone config. However, the config provided by command line still cannot override values in standalone config. The priority between repo-config.csv
and the command line is not decided, too.
The current priority order is a bit odd. How about this?
The normal priority order CLI > report config > standalone config
This means report config files will override standalone config by default. However, the report config can allow the standalone config to override its values using the overridable:
prefix.
Is that a workable approach?
The current priority order is a bit odd. How about this? The normal priority order CLI > report config > standalone config
The order makes sense.
This means report config files will override standalone config by default. However, the report config can allow the standalone config to override its values using the
overridable:
prefix.
Also workable. We can change the ignore standalone config
to adopt standalone config
which means we use overridable:
prefix for all columns.
Is that a workable approach?
Under this circumstance, when CLI argument exists, it will override the same argument in both standalone config
and report config file
. Meanwhile, the meaning of --ignore
should change to whether to only use the argument provided by CLI or also use the argument provided by standalone config
or report config file
which is not included in CLI. (i.e. ignoreGlobList
and ignoreCommitList
)
I hope my explanation is clear enough, lol.
sounds good.
We can change --ignore-standalone-config
to --apply-standalone-config
/-a
to mean apply standalone config. What do you think?
sounds good. We can change
--ignore-standalone-config
to--apply-standalone-config
/-a
to mean apply standalone config. What do you think?
For this apply
, do you mean that the rest field in standalone config can be used or the field can override the value in CLI?
The normal priority order CLI > report config > standalone config This means report config files will override standalone config by default. However, the report config can allow the standalone config to override its values using the overridable: prefix.
The order is better, however, wouldn't this means that if I were want to ignore a specific field in standalone config, I will need to fill up all the empty fields with the overridable:
prefix?
The order is better, however, wouldn't this means that if I were want to ignore a specific field in standalone config, I will need to fill up all the empty fields with the
overridable:
prefix?
Yes, I suppose so. It's inconvenient, but can tolerate. Not that difficult to copy-paste a value into a range of cells in excel. Any better alternatives?
For this
apply
, do you mean that the rest field in standalone config can be used or the field can override the value in CLI?
It can mean the standalone overrides csv, not CLI. As -a
is a CLI flag, it should not override CLI parameters (i.e., one flag in the command should not override other parts of the same command).
Any better alternatives?
I am thinking of having 2 different mode, inherit
and default
.
inherit
will take the values from standalone config, while
default
uses the default value defined by the application.
This is to define the action for the application to take, should the field in csv is empty.
When using the flag --inherit
(which can also be -a
), it will automatically use the values of the standalone config when the csv field is empty.
While default
can just be default, where application just the default value defined by the application, hence there is no need to have a dedicated flag for it.
If the user truly wants a specific field to be empty, he can do it using a dedicated constant e.g. ""
or empty
.
How does that sound?
We'd better define all possibilities.
-
--inherit
- standalone missing, csv present:
- standalone present, csv missing:
- both absent:
- both missing:
- default
- standalone missing, csv present:
- standalone present, csv missing:
- both absent:
- both missing:
In both cases, CSV has highest priority, hence:
- --inherit
- standalone missing, csv present: use CSV values, define blank fields with default value
- standalone present, csv missing: inherits all standalone values
- both present: use CSV values, empty fields will be obtained from standalone config.
- both missing: use default value
- default
- standalone missing, csv present: use CSV values, define blank fields with default value
- standalone present, csv missing: use default value
- both present: use CSV values, define blank fields with default value
- both missing: use default value
So in summary, --inherit
shall use standalone config values whenever possible when it's not defined in CSV; CSV will still has the highest priority.
Sounds good. Minor change given below.
--inherit
- both present: use CSV values, empty fields will be obtained from standalone config. use
""
in csv file to use default instead of standalone
To summarise the discussion above,
-
CLI will have higher priority than standalone config --> If a user specifies the same argument on CLI and in standalone config, the value on the CLI takes effect. Currently, the only common argument in CLI and standalone config is
formats
. -
CLI will have higher priority than CSV Configs. Currently, the only common arguments between CLI and the CSV Configs are
formats
andignore-standalone-config
. -
CSV Configs will have higher priority than standalone config. Currently, the common arguments are
formats
,ignore commit list
,ignore globs list
and the author configurations. However, we can use theinherit
mechanism to allow standalone config to fill in the empty fields in the config csv.
May I clarify the following:
-
Is it right that, with the
inherit / default
mechanism, we no longer need theignore standalone config
column inrepo-config.csv
? My reasoning: if we have N repos inrepo-config.csv
, and we wish to ignore the standalone config only for some of them, then we just need to fill out all the fields for these repositories and useinherit
mode. This will allow the remaining repos to continue using their standalone configs. -
Is it right that these changes in priority do not affect the
--ignore-standalone-config
CLI option ? We still need a way for users to disable the standalone config when they are specifying repositories via--repo
Good question
Is it right that, with the inherit / default mechanism, we no longer need the ignore standalone config column in repo-config.csv ? My reasoning: if we have N repos in repo-config.csv, and we wish to ignore the standalone config only for some of them, then we just need to fill out all the fields for these repositories and use inherit mode. This will allow the remaining repos to continue using their standalone configs.
Note that theres author configurations involve as well, that is something that need to be take care of as well.
Is it right that these changes in priority do not affect the --ignore-standalone-config CLI option ? We still need a way for users to disable the standalone config when they are specifying repositories via --repo
Yes
Note that theres author configurations involve as well, that is something that need to be take care of as well.
Wouldn't it follow the same logic?
-
--inherit
- standalone missing,
author-config.csv
present: Get author configurations from CSV. If author configurations for a particular repository are not present in the CSV, then use all authors within the date range. - standalone present,
author-config.csv
missing: Get author configurations from standalone config. If author configurations are not present in standalone, use all authors within the date range. - both present: Get author configurations from CSV. If author configurations for a particular repository are not present in CSV, use that repository's standalone config. If there are no author configurations in that repository's standalone config, use all authors within date range.
- both missing: Use all authors within date range.
- standalone missing,
-
default
- standalone missing,
author-config.csv
present: Get author configurations from CSV. If author configurations for a particular repository are not present, then use all authors within the date range. - standalone present,
author-config.csv
missing: Use all authors within the date range. - both present: Use author configurations in CSV. If author configurations for a particular repository are not present, use all authors within the date range.
- both missing: Use all authors within the date range.
- standalone missing,
Wouldn't it follow the same logic?
Do note that the complication here is, how are you going to enforce the use of all authors / override when the --inherit
mode is present?