Skip to content
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

Fix not ignoring restG4 library on restRoot #299

Merged
merged 2 commits into from
Sep 14, 2022
Merged

Conversation

lobis
Copy link
Member

@lobis lobis commented Sep 13, 2022

lobis Ok: 3

Recently TRestTools was mofidied (#295) to fix an error on the GitLab pipeline. The fix removed the case insensitive flag for the comparison of the ignored libraries (since we didn't remember why it was there in the first place) but this breaks the ignore of restG4 library.

The library is being loaded (altought it doesn't seem to cause any issue, just a warning).

This is because the library name contains RestG4, not restG4. This name is generated from the package name (restG4) which is hardcoded into TRestTools. We could just use RestG4 instead of restG4 but I think we should instead use the case insensitive comparison so in the future if we can get all the package names automatically instead of hardcoding this will work without modifications.

@lobis lobis added the bug Something isn't working label Sep 13, 2022
@jgalan
Copy link
Member

jgalan commented Sep 14, 2022

if we can get all the package names automatically instead of hardcoding this will work without modifications.

Still you will need to do modifications for the automatic approach. So, writting the name to be excluded as RestG4 seems more restrictive, and thus lower probability to get into errors again.

If you ignore case, it might find again restg4 in somewhere and fail.

@lobis
Copy link
Member Author

lobis commented Sep 14, 2022

if we can get all the package names automatically instead of hardcoding this will work without modifications.

Still you will need to do modifications for the automatic approach. So, writting the name to be excluded as RestG4 seems more restrictive, and thus lower probability to get into errors again.

If you ignore case, it might find again restg4 in somewhere and fail.

Okay I applied the changes, we can merge now I think

@lobis lobis requested a review from Oscar-PL September 14, 2022 10:39
@lobis lobis merged commit 788f7a5 into master Sep 14, 2022
@lobis lobis deleted the lobis-ignore-library-fix branch September 14, 2022 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants