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: log the checked files if verbose #924

Closed
wants to merge 1 commit into from

Conversation

aminya
Copy link
Contributor

@aminya aminya commented Nov 4, 2022

This adds a logging step to the function that checks if the inputs/generates are up to date. After hours of debugging, It helped me to finally detect that the globbing library that task uses fails to check some patterns.

The following checks the files inside the dev folder

"{dev/**/*,something}"

But this doesn't!

"{something,dev/**/*}"

Or for example, ** doesn't check all the files of the current project.

Performance-wise, this is not a good solution because it calls the glob outside the checker.IsUpToDate function. I did this because that was the only way I could access the logger without changing the checker API. If you are OK with the API change, we can remove the duplicate glob calls.

@andreynering
Copy link
Member

Hi @aminya, thanks for creating this PR.

I gave this some thought and decided to close because:

  1. sources/generates can be a lot a files (if **/*.go in a big repo for example) and can potentially generate a lot of noise in the logs, which could annoy users debugging other stuff
  2. We plan to add another feature to allow this kind of debugging. We'll probably have a --debug flag some day to output useful data about the Taskfiles, like computed variables, sources, generates, and more.

@andreynering
Copy link
Member

I just opened #931 to track this idea.

@aminya
Copy link
Contributor Author

aminya commented Nov 12, 2022

Thanks. Could you please make an issue for the Glob errors too? The glob package that you use is not validated

@aminya aminya mentioned this pull request Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants