Skip to content

Conversation

aoudiamoncef
Copy link
Contributor

No description provided.

dmlloyd
dmlloyd previously requested changes Nov 7, 2019
@aoudiamoncef
Copy link
Contributor Author

@dmlloyd ,
What do you think if we choose a unified Logger name convention ? i found:
private static final LOGGER, logger, LOG, log
and different kind of instantiations:
Logger.getLogger with Test.class, Test.class.getName(), "Test"

@dmlloyd
Copy link
Member

dmlloyd commented Nov 7, 2019

I don't like using the class name as the log category. But I also don't like arguing about it very much! I would recommend using a category name that groups together messages that relate to whatever process is being executed using Logger.getLogger(String). If people insist on using the class name, then Logger.getLogger(Class<?>) is the appropriate way to do it.

@aoudiamoncef
Copy link
Contributor Author

Thanks for your feedback. Do you think that failing tests are caused by the changes in this PR ?

@machi1990
Copy link
Member

Thanks for your feedback. Do you think that failing tests are caused by the changes in this PR ?

I am seeing this is error

Caused by: java.lang.ClassFormatError: Duplicate method name "findById" with signature "(Ljava.lang.Object;)Ljava.lang.Object;" in class file io/quarkus/it/mongodb/panache/bugs/Bug5274EntityRepository
	at java.lang.ClassLoader.defineClass1(Native Method)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:756)
	at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
	at java.net.URLClassLoader.defineClass(URLClassLoader.java:468)
	at java.net.URLClassLoader.access$100(URLClassLoader.java:74)
	at java.net.URLClassLoader$1.run(URLClassLoader.java:369)
	at java.net.URLClassLoader$1.run(URLClassLoader.java:363)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.net.URLClassLoader.findClass(URLClassLoader.java:362)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:418)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:352)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:351)
	at io.quarkus.it.mongodb.panache.bugs.Bug5274EntityRepository_Bean.<init>(Bug5274EntityRepository_Bean.zig:143)
	at io.quarkus.arc.setup.Default_ComponentsProvider.addBeans1(Default_ComponentsProvider.zig:227)
	at io.quarkus.arc.setup.Default_ComponentsProvider.getComponents(Default_ComponentsProvider.zig:38)
	at io.quarkus.arc.impl.ArcContainerImpl.<init>(ArcContainerImpl.java:102)
	at io.quarkus.arc.Arc.initialize(Arc.java:19)
	at io.quarkus.arc.runtime.ArcRecorder.getContainer(ArcRecorder.java:34)
	at io.quarkus.deployment.steps.ArcProcessor$generateResources15.deploy_0(ArcProcessor$generateResources15.zig:72)
	at io.quarkus.deployment.steps.ArcProcessor$generateResources15.deploy(ArcProcessor$generateResources15.zig:36)
	at io.quarkus.runner.ApplicationImpl.<clinit>(ApplicationImpl.zig:296)

I remember @FroMage fixed it recently here #5276.

@FroMage
Copy link
Member

FroMage commented Nov 8, 2019

WTH? This test passed when we merged it, no?

@machi1990
Copy link
Member

WTH? This test passed when we merged it, no?

Yeah, CI was happy when it was merged.

@FroMage
Copy link
Member

FroMage commented Nov 8, 2019

Just tried it on master and it passes.

@FroMage
Copy link
Member

FroMage commented Nov 8, 2019

Perhaps caching went wrong and it was using an older version of mongo-panache, let's restart the test.

@FroMage
Copy link
Member

FroMage commented Nov 8, 2019

Single job restart still failed, probably didn't wipe the faulty cache. Just restarted all jobs.

@FroMage
Copy link
Member

FroMage commented Nov 8, 2019

So the job passed, but it's not reflected here: https://dev.azure.com/quarkus-ci/quarkus/_build/results?buildId=11841

@geoand
Copy link
Contributor

geoand commented Nov 9, 2019

Has the PR been rebased onto the latest master that fixes CI?

@FroMage
Copy link
Member

FroMage commented Nov 9, 2019

What part of master fixed what part of CI?

@geoand
Copy link
Contributor

geoand commented Nov 9, 2019

I have no idea since I've been away the past few days.
I just saw various PRs this morning and they all had passing CI.

@aoudiamoncef aoudiamoncef force-pushed the master branch 3 times, most recently from bbc98c7 to bf95718 Compare November 12, 2019 20:32
@aoudiamoncef
Copy link
Contributor Author

@dmlloyd ,
All seems to be ok now :)

@machi1990 machi1990 requested a review from dmlloyd November 13, 2019 12:05
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.

6 participants