-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add enforcer rule to detect if extension misses description in pom #6335
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the description of the Hibernate ORM extension (the one in the runtime artifact) and the build didn't fail.
I checked the parent of this extension in case it would have a meaningful description but AFAICS it doesn't.
Did I miss something?
@gastaldi have you tested this PR before approving? Because I can't get it to fail as expected. |
@gastaldi if so, can you tell me which extension you tested so that I can check what's different with the one I tested. Thanks. |
@gsmet Yes, I've tested before approving it.
|
@gsmet the same happens with
|
Weird I tested on qute and arc. All failed when I removed the description. |
@gastaldi yeah, that's why I asked. It looked suspicious as the code looked right. I checked several times I had the commit and that the description was gone. I will check again. |
So... in the end everyone is right:
Can one of you check my observations? |
Weird. I tested full build with missing description and failed for me. I'll try when back at laptop after Xmas. |
Yes, we're not in a hurry. I checked another time dropping the JGit + Hibernate ORM descriptions and the build passes. |
xmas dinner over; can confirm I see the behaviour @gsmet sees too. investigating. |
the check for quarkus-extension.yaml was not using project.base.dir resulting it to only work when done within a runtime dir. added project.basedir so it should now work. I must have done the multimodule tests before I added that check, sorry about that; but now @gsmet\s test case should definitely fail with my latest push. |
84d034a
to
5f9acc4
Compare
@maxandersen the rule seems OK now. I pushed some additional changes to fix the issues it reported (GH doesn't respect the commit ordering but I moved them before the rule addition so that the build doesn't get broken). |
I tried to fix the Windows issue but I couldn't find a solution:
From what I can guess, the issue is that the generated We would need another way to get the root but BeanShell support is very poorly documented and from the source, I don't see any way to access the Maybe we should develop a plain old Java rule but I suspect that it will require some infrastructure? Well, it's time for my "I told you so" ;). |
told me so what ? I said several times we would need a custom rule for this quarkus repo specific need. this is something unique for quarkus project layout. I'll try and make it work on windows; if not we can make a custom rule as indepdent project; shouldn't be a big deal (I hope ;) |
I'm closing this as it doesn't work. We would need another approach if we want to pursue this. |
adds simple enforcer check if build has an extensions.yaml it should at least also have a proper description in pom.xml that is not similar to what is in the parent.