Skip to content

Forge 842 #306

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Forge 842 #306

wants to merge 1 commit into from

Conversation

salmon-charles
Copy link

Please note that the followings junit errors:

forge-project-model-maven-junittests-failures

do not seem to come from my fix.

gastaldi
gastaldi reviewed Apr 5, 2013
View reviewed changes
project-model-maven/src/main/java/org/jboss/forge/maven/facets/MavenPluginFacetImpl.java Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we use this pattern for naming private methods

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we don't :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, while it's not a huge deal, it would probably be good to remove the __s :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this looks great, and thanks for letting us know about the tests!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, you are absolutely right !
I often use the "__" for privates methods when doing object-oriented javascript.
I'll will fix it and remake the pull.

Charles.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! We'll merge it right after that! Good job

Em sexta-feira, 5 de abril de 2013, salmon-charles escreveu:

In
project-model-maven/src/main/java/org/jboss/forge/maven/facets/MavenPluginFacetImpl.java:

     return plugins;
 }

  • @OverRide
  • public void addPlugin(final MavenPlugin plugin) {
  • private void __addPlugin(final MavenPlugin plugin, boolean managedPlugin) {

Oops, you are absolutely right !
I often use the "__" for privates methods when doing object-oriented
javascript.
I'll will fix it and remake the pull.

Charles.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/306/files#r3678446
.

@salmon-charles
Copy link
Author

Humm, something wrong with my commit; I should have only 4 Files changed.
I am new with github and I have to go ; I will let you know when everything is OK.

Charles.

@salmon-charles
Copy link
Author

Here we go !
Please let me know if there are further problems.

@gastaldi
Copy link
Member

gastaldi commented Apr 5, 2013

No problem. Check
http://forge.jboss.org/docs/get_involved/contribute.html#content if you
need some info. I can do it also if you prefer

Em sexta-feira, 5 de abril de 2013, salmon-charles escreveu:

Humm, something wrong with my commit; I should have only 4 Files changed.
I am new with github and I have to go ; I will let you know when
everything is OK.

Charles.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/306#issuecomment-15975644
.

@gastaldi
Copy link
Member

gastaldi commented Apr 5, 2013

You need to use the default Forge code formatter in future patches also. II'll reformat them for you in this patch

@gastaldi
Copy link
Member

gastaldi commented Apr 5, 2013

A Test failure occurred due to these changes :

java.lang.RuntimeException: Configuration 'proc' not found
    at org.jboss.forge.maven.plugins.ConfigurationImpl.getConfigurationElement(ConfigurationImpl.java:57)
    at org.jboss.forge.spec.PersistenceMetaModelFacetTest.testInstallFacet(PersistenceMetaModelFacetTest.java:47)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
    at org.jboss.arquillian.junit.Arquillian$6$1.invoke(Arquillian.java:270)
    at org.jboss.arquillian.container.test.impl.execution.LocalTestExecuter.execute(LocalTestExecuter.java:60)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:90)
    at org.jboss.arquillian.core.impl.EventContextImpl.invokeObservers(EventContextImpl.java:99)
    at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:81)
    at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:134)
    at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:114)
    at org.jboss.arquillian.core.impl.EventImpl.fire(EventImpl.java:67)
    at org.jboss.arquillian.container.test.impl.client.protocol.local.LocalContainerMethodExecutor.invoke(LocalContainerMethodExecutor.java:50)
    at org.jboss.arquillian.container.test.impl.execution.RemoteTestExecuter.execute(RemoteTestExecuter.java:120)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:90)
    at org.jboss.arquillian.core.impl.EventContextImpl.invokeObservers(EventContextImpl.java:99)
    at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:81)
    at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:134)
    at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:114)
    at org.jboss.arquillian.core.impl.EventImpl.fire(EventImpl.java:67)
    at org.jboss.arquillian.container.test.impl.execution.ClientTestExecuter.execute(ClientTestExecuter.java:57)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:90)
    at org.jboss.arquillian.core.impl.EventContextImpl.invokeObservers(EventContextImpl.java:99)
    at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:81)
    at org.jboss.arquillian.container.test.impl.client.ContainerEventController.createContext(ContainerEventController.java:142)
    at org.jboss.arquillian.container.test.impl.client.ContainerEventController.createTestContext(ContainerEventController.java:129)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:90)
    at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:88)
    at org.jboss.arquillian.test.impl.TestContextHandler.createTestContext(TestContextHandler.java:89)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:90)
    at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:88)
    at org.jboss.arquillian.test.impl.TestContextHandler.createClassContext(TestContextHandler.java:75)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:90)
    at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:88)
    at org.jboss.arquillian.test.impl.TestContextHandler.createSuiteContext(TestContextHandler.java:60)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.jboss.arquillian.core.impl.ObserverImpl.invoke(ObserverImpl.java:90)
    at org.jboss.arquillian.core.impl.EventContextImpl.proceed(EventContextImpl.java:88)
    at org.jboss.arquillian.core.impl.ManagerImpl.fire(ManagerImpl.java:134)
    at org.jboss.arquillian.test.impl.EventTestRunnerAdaptor.test(EventTestRunnerAdaptor.java:111)
    at org.jboss.arquillian.junit.Arquillian$6.evaluate(Arquillian.java:263)
    at org.jboss.arquillian.junit.Arquillian$4.evaluate(Arquillian.java:226)
    at org.jboss.arquillian.junit.Arquillian.multiExecute(Arquillian.java:314)
    at org.jboss.arquillian.junit.Arquillian.access$100(Arquillian.java:46)
    at org.jboss.arquillian.junit.Arquillian$5.evaluate(Arquillian.java:240)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
    at org.jboss.arquillian.junit.Arquillian$2.evaluate(Arquillian.java:185)
    at org.jboss.arquillian.junit.Arquillian.multiExecute(Arquillian.java:314)
    at org.jboss.arquillian.junit.Arquillian.access$100(Arquillian.java:46)
    at org.jboss.arquillian.junit.Arquillian$3.evaluate(Arquillian.java:199)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
    at org.jboss.arquillian.junit.Arquillian.run(Arquillian.java:147)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)

@gastaldi
Copy link
Member

gastaldi commented Apr 5, 2013

Fixed. It was because the pom is regenerated every time mavenCoreFacet.getPom() Is called. Running tests and will merge as soon as it passes. Thanks !

@gastaldi
Copy link
Member

gastaldi commented Apr 5, 2013

Merged. Thanks!

@gastaldi gastaldi closed this Apr 5, 2013
@gastaldi
Copy link
Member

gastaldi commented Apr 6, 2013

Yes, you can install the Code formatter in Eclipse in
https://github.com/forge/core/blob/master/eclipse-code-formatter-profile.xml

I think we missed this info in the website. We'll fix it asap.

Thanks once again!

2013/4/5 salmon-charles notifications@github.com

Great, thanks !
By the way, what is the "default Forge code formatter" ?
Is there some documentation somewhere ?
Something I can install on my Eclipse to ensure the code is formatted as
required ?

Thanks,
Charles.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/306#issuecomment-15987252
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@salmon-charles @gastaldi @lincolnthree