[GitHub] [maven-build-cache-extension] kbuntrock commented on pull request #91: [MBUILDCACHE-64] Exclusion mechanism bugfix
kbuntrock commented on PR #91: URL: https://github.com/apache/maven-build-cache-extension/pull/91#issuecomment-1739027006 About this PR, I'm still working on it. Even if I couldn't find the time the last past weeks. I re-put it in draft @maximilian-novikov-db et @AlexanderAshitkin, I think I'll be able to submit a more consensual version in the next days. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-build-cache-extension] kbuntrock commented on pull request #91: [MBUILDCACHE-64] Exclusion mechanism bugfix
kbuntrock commented on PR #91: URL: https://github.com/apache/maven-build-cache-extension/pull/91#issuecomment-1695606322 > I'm starting to wonder if the use of the JDK `PathMatcher` is sufficient here. If we want to optimize the file system access and not traverse _all_ files / subdirectories, we'll have to use [`Files.walkFileTree`](https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#walkFileTree-java.nio.file.Path-java.nio.file.FileVisitor-) as it has built-in support for skipping subtrees. However, I'm not sure there is any good implementation of glob syntax that would skip subtree when known to not match. Well, good news, we are already using `Files#walkFileTree`. One of the discussion is precisely how I managed to keep as well as possible the capacity to skip subtree scanning (it is a key point for the projects I want to use this extension on). @All : Seems this PR draw attention to details about this functionality but I would suggest to not go too far with it. I already agreed to change it from a bugfix to an evolution. Maybe the current state is good enough for now. And if good ideas emerge they can be implemented in another PR. I have the feeling that in term of functionality / performance / test and documentation, this PR brings already a lot to the table. And people will be easily capable after merging to change some details they would have seen differently. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-build-cache-extension] kbuntrock commented on pull request #91: [MBUILDCACHE-64] Exclusion mechanism bugfix
kbuntrock commented on PR #91: URL: https://github.com/apache/maven-build-cache-extension/pull/91#issuecomment-1690697801 > @kbuntrock nicely done, it would be useful to update the sample config as well https://github.com/apache/maven-build-cache-extension/blob/master/src/site/resources/maven-build-cache-config.xml Done it. Wasn't comfortable with this file since I found it confusing when I discovered this extension + it was still valid with my changes. Added some comments on some related other parameters around. Is it ok for you @maximilian-novikov-db ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-build-cache-extension] kbuntrock commented on pull request #91: [MBUILDCACHE-64] Exclusion mechanism bugfix
kbuntrock commented on PR #91: URL: https://github.com/apache/maven-build-cache-extension/pull/91#issuecomment-1668279033 Changed the exclude mechanism to use path matchers. A bit more than a bugfix now, feedbacks appreciated if you see any lack, implementation detail you disagree with or extra test case to add. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-build-cache-extension] kbuntrock commented on pull request #91: [MBUILDCACHE-64] Exclusion mechanism bugfix
kbuntrock commented on PR #91: URL: https://github.com/apache/maven-build-cache-extension/pull/91#issuecomment-1665985477 > I'm a bit skeptical about the way which has been chosen to filter files. I think it would be more intuitive to use a glob `PathMatcher` rather than a `startsWith()` call on the path. This seems inconsistent with what is usually done in the maven land, for example https://maven.apache.org/plugins/maven-jar-plugin/examples/include-exclude.html Sure, I can do this. It will also bring a bit more flexibility. I suggest that I stay focused on the exclude section and I don't touch the input section. PS : Since I'm new there, hello everyone and great job! This project is a really good idea. I'm dying to be able to use it on my projects. :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [maven-build-cache-extension] kbuntrock commented on pull request #91: [MBUILDCACHE-64] Exclusion mechanism bugfix
kbuntrock commented on PR #91: URL: https://github.com/apache/maven-build-cache-extension/pull/91#issuecomment-1664633394 Miss-clicked on the sync button on my repo for this branch (I only wanted to sync master ...). So I finished to rebase it on master to keep the tree clean. But it reseted the workflow, sorry. :s -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
