Some CS2103 commits not showing up in the reposense dashboard

[damithc]

Bug reported by student https://github.com/nus-cs2103-AY2021S2/forum/issues/302 Config https://github.com/nus-cs2103-AY2021S2/tp-dashboard/tree/master/configs Dashboard https://nus-cs2103-ay2021s2.github.io/tp-dashboard Commits by author (noelmathewisaac)

Need to fix quickly if this is a real problem

[damithc]

@gerhean @dcshzj @HCY123902 need help with this

[HCY123902]

After trying it locally, removing src/main/resources in the Ignore Glob List column in repo-config.json seems to make the report include commits that only involves changes with Fxml and CSS files. The possible cause of the original problem is that the commits that only involve the files residing in src/main/resources are not recognized by the report

[damithc]

Thanks for the investigation @HCY123902 How come .fxml files still appear in the code view? Perhaps 'ignore glob' is not working fully?

[HCY123902]

I think using src/main/resources/** instead of src/main/resources will remove the Fxml file changes and commits that only involves Fxml and CSS files from the report, which should give the intended behavior. The current issue seems to be that, when path glob such as src/main/resources is used in the column, some file changes in the path glob are still captured by the authorship contribution panel, but the commits that only involve changes in the file path glob will be ignored by the report. Maybe the documentation should include the file path glob format specification in the CSV configuration?

[damithc]

I think using src/main/resources/** instead of src/main/resources will remove the Fxml file changes and commits that only involves Fxml and CSS files from the report, which should give the intended behavior. The current issue seems to be that, when path glob such as src/main/resources is used in the column, some file changes in the path glob are still captured by the authorship contribution panel, but the commits that only involve changes in the file path glob will be ignored by the report. Maybe the documentation should include the file path glob format specification in the CSV configuration?

Thanks for the further investigation @HCY123902 Yes, we can update the documentation if necessary.

[gerhean]

Should we remove src/main/resources/view from the Ignore Glob List? So something like src/main/resources/!(view)/**?

Or is it intended that UI changes should not be tracked?

[damithc]

Should we remove src/main/resources/view from the Ignore Glob List? So something like src/main/resources/!(view)/**?

Can try that. Confirmed that's the correct glob syntax?

[damithc]

Should we remove src/main/resources/view from the Ignore Glob List? So something like src/main/resources/!(view)/**?

Can try that. Confirmed that's the correct glob syntax?

Seems to work. Thanks for the top @gerhean We can add a few examples like these in the documentation.

[gerhean]

I just assumed that the syntax is the same as the frontend minimatch, but I can't say for sure whether the backend uses a package which uses the same syntax to parse.

Also I think src/main/resources/!(view)/** does not match files placed directly in the resource folder (means reposense will not ignore them), not sure if that's important.

[damithc]

Also I think src/main/resources/!(view)/** does not match files placed directly in the resource folder (means reposense will not ignore them), not sure if that's important.

I'm also not sure if this expression is a good fit, although doesn't seem to break anything ATM. In any case, we should provide better documentation of glob patterns supported. Might depend on the libraries used by the frontend and backend as globs are used in both parts.

[gerhean]

This is fairly comprehensive https://mywiki.wooledge.org/glob

[damithc]

This is fairly comprehensive https://mywiki.wooledge.org/glob

Nice. Let's point to it from our UG.

[HCY123902]

Actually, there is already a link in User Guide pointing to https://docs.oracle.com/javase/tutorial/essential/io/fileOps.html#glob. It is just that it is not obvious enough in that section

[HCY123902]

@gerhean Do you mind if I start a PR on it? I think it should just be a minor change that adds some link to the User Guide

[dcshzj]

#1541 has been merged, any other tasks still pending for this issue?

[yhtMinceraft1010X]

Propose closing this issue since the corresponding issue in the CS2103 forum for that semester has also been closed (and something from 2 years back is unlikely to be relevant today).

[chan-j-d]

I agree, think we can close the issue