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

[wokdlhX3] Extract code from APOC core which is only used in APOC extended #3361

Merged
merged 11 commits into from
Jan 16, 2023

Conversation

Lojjs
Copy link
Contributor

@Lojjs Lojjs commented Dec 9, 2022

Should go in together with neo4j/apoc#261

Copy link
Collaborator

@conker84 conker84 left a comment

Choose a reason for hiding this comment

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

Thank you so much @Lojjs, just a couple of things that I want to discuss with you

.map(PathResult::new);
}

private static final class VirtualPathBuilder {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to leave the API in VirtualPath in core

@@ -87,7 +88,8 @@ private String[] getHeader(CSVReader csv, LoadCsvConfig config) throws IOExcepti
Map<String, Mapping> mappings = config.getMappings();
for (int i = 0; i < headers.length; i++) {
String header = headers[i];
if (ignore.contains(header) || mappings.getOrDefault(header, Mapping.EMPTY).ignore) {
Mapping empty = new Mapping("", Collections.emptyMap(), LoadCsvConfig.DEFAULT_ARRAY_SEP, false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe adding it as a static final?

@Lojjs Lojjs force-pushed the dev-move-parts-of-common-from-core branch from de07bcf to 830df13 Compare January 2, 2023 11:59
@Lojjs Lojjs force-pushed the dev-move-parts-of-common-from-core branch from b595562 to 55fbd1c Compare January 2, 2023 13:52
Copy link
Collaborator

@conker84 conker84 left a comment

Choose a reason for hiding this comment

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

@Lojjs LGTM thank you so much!

@Lojjs Lojjs merged commit 4044f23 into dev Jan 16, 2023
@Lojjs Lojjs deleted the dev-move-parts-of-common-from-core branch January 16, 2023 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-cypher-surface Cypher Surface team should review the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants