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

NO AUTO Does a tidy-up and clean-up after splitting most of the code #3123

Merged
merged 1 commit into from Aug 11, 2022

Conversation

ncordon
Copy link
Contributor

@ncordon ncordon commented Aug 9, 2022

  • Moves more files to full (the ones that are only needed there and not in core)
  • Tidy ups dependencies: there were lots of unused things.
  • Removes sonarqube plugin. I don't think this was used anymore, this is from the old days when the code was published to Sonar rather than Maven.

@ncordon ncordon added 5.0 team-cypher-surface Cypher Surface team should review the PR labels Aug 9, 2022
@ncordon ncordon force-pushed the dev-split-cleanup branch 2 times, most recently from 1591d60 to 5b9fc3e Compare August 9, 2022 16:13
testImplementation group: 'org.eclipse.jetty', name: 'jetty-server', version: '9.2.22.v20170606'
testImplementation group: 'org.apache.derby', name: 'derby', version: '10.12.1.1'

testImplementation group: 'org.postgresql', name: 'postgresql', version: '42.1.4'
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like a test for the Postgres driver is failing for a genuine reason. Could be that this Postgres dependency is needed?

You may wish to run the tests locally if you haven't already, since many of these types of tests are probably disabled in the CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep good catch, I've brought back this, but I don't understand how the code compiles if we need it in runtime

});
}

}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you may have wanted to delete the src/test/Examples.java instead of the src/test/ExamplesTest.java?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch man

@@ -71,7 +70,7 @@ subprojects {

// neo4jDockerImage system property is used in TestContainerUtil
systemProperties 'user.language' : 'en' ,
'user.country ' : 'US',
'user.country' : 'US',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was spotted by @AzuObs, not me

Copy link
Contributor

@AzuObs AzuObs left a comment

Choose a reason for hiding this comment

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

Great job on this! 🙌

@ncordon ncordon merged commit 4899849 into dev Aug 11, 2022
@ncordon ncordon deleted the dev-split-cleanup branch August 11, 2022 09:20
@ncordon ncordon added the split Work related to splitting apoc core and full label Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 split Work related to splitting apoc core and full team-cypher-surface Cypher Surface team should review the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants