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

Prevent failure of DocumentContentSynchronizer.<init> #841

Merged

Conversation

rubenporras
Copy link
Contributor

@rubenporras rubenporras commented Oct 6, 2023

Prevent failure of DocumentContentSynchronizer. if the document
does not have an URI registered in EFS and it is not a file uri.

Also improve the existing log to include the URI that we are failing to
deal with.

@rubenporras
Copy link
Contributor Author

I wonder if we actually need a warning or if we just can take 0L as default without bothering the user with a warning. What do you think @mickaelistria ?

@mickaelistria
Copy link
Contributor

I don't think I ever encountered this use-case, so I don't have anything relevant to answer here. I think this patch is fine as it, so if you think it's worth merging it now, please do. If you think it's better to not log, then don't log and merge it. I fully trust your judgement here ;)

does not have an URI registered in EFS and it is not a file uri.

Also improve the existing log to include the URI that we are failing to
deal with.
@rubenporras
Copy link
Contributor Author

Thanks, I think the code can handle the case quite well by taking the default value of 0L, so I will not give the warning.

@rubenporras rubenporras merged commit 0e73a30 into eclipse:master Oct 9, 2023
1 of 2 checks passed
@rubenporras rubenporras deleted the DocumentContentSynchronizer branch October 9, 2023 07:07
@rubenporras rubenporras changed the title Document content synchronizer Prevent failure of DocumentContentSynchronizer.<init> Oct 20, 2023
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.

None yet

2 participants