Skip to content

Conversation

@amit-jain
Copy link
Contributor

…aths added and preserve sub-paths

  • Added an optional consumer to be notified of any added paths
  • Create a new preserveOnTarget which does not delete any sub-paths on target

…aths added and preserve sub-paths

- Added an optional consumer to be notified of any added paths
- Create a new preserveOnTarget which does not delete any sub-paths on target
@amit-jain amit-jain requested review from mbaedke and mreutegg June 23, 2022 06:37
@amit-jain
Copy link
Contributor Author

test failure in org.apache.jackrabbit.j2ee.TomcatIT due to timeouts and doesn't look related to the changes here

java.lang.RuntimeException: java.net.SocketTimeoutException: Read timed out
at com.gargoylesoftware.htmlunit.WebClient.download(WebClient.java:2378)

@amit-jain amit-jain requested a review from trekawek June 28, 2022 08:17
Copy link
Contributor

@trekawek trekawek left a comment

Choose a reason for hiding this comment

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

LGTM. Let's just make it clear what the new consumer accepts.

boolean referenceableFrozenNodes) {
boolean referenceableFrozenNodes,
boolean preserveOnTarget,
Consumer<String> consumer) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's call is newNodesConsumer or something like that.

…aths added and preserve sub-paths

Review changes
- Rename consumer variable to make clear intention of consuming added paths only
@amit-jain amit-jain merged commit 5e1f45c into apache:trunk Jun 29, 2022
rishabhdaim pushed a commit to rishabhdaim/jackrabbit-oak that referenced this pull request Jul 4, 2022
apache#603)

- Added an optional consumer to be notified of any added paths
- Create a new preserveOnTarget which does not delete any sub-paths on target
- Rename consumer variable to make clear intention of consuming added paths only
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.

2 participants