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

Fix mismatched begin/endOperation #475

Merged
merged 1 commit into from
Jun 9, 2023
Merged

Conversation

jukzi
Copy link
Contributor

@jukzi jukzi commented Jun 2, 2023

Every prepareOperation() needs a final(!) endOperation() see https://bugs.eclipse.org/bugs/show_bug.cgi?id=124211#c4

In case of Exceptions an endOperation() could have been missing.

@mickaelistria
Copy link
Contributor

I don't think the changes in WorkManager are related to the initial intent of this patch. Can you please create a separate PR for those?

@jukzi
Copy link
Contributor Author

jukzi commented Jun 2, 2023

It relates: i made the WorkManager methods package private so that i am sure i hit all references - especially to checkOut().

@mickaelistria
Copy link
Contributor

It relates: i made the WorkManager methods package private so that i am sure i hit all references - especially to checkOut().

I see it more as a preliminary "technical" change towards elaborating the proper "functional" fix, but not a requirement.
There are risks that someone does use the WorkManager directly despite it's internal, so there are risk that we will have to revert it because of an important enough use-case and incapacity to address it better in a good time frame. So if we can have 2 distinct commits, it would allow to revert -if necessary- one of them without reverting the other thus to keep value in.
1 PR with 2 commits would be OK too.

@jukzi
Copy link
Contributor Author

jukzi commented Jun 2, 2023

ok, i will do

Every prepareOperation() needs a final(!) endOperation()
see https://bugs.eclipse.org/bugs/show_bug.cgi?id=124211#c4

In case of Exceptions an endOperation() could have been missing.
@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2023

Test Results

     28 files  ±0       28 suites  ±0   40m 25s ⏱️ - 5m 51s
2 380 tests ±0  2 377 ✔️  - 1  1 💤 ±0  1 +1  1 🔥 ±0 
6 999 runs  ±0  6 993 ✔️  - 1  4 💤 ±0  1 +1  1 🔥 ±0 

For more details on these failures and errors, see this check.

Results for commit b1e6fe4. ± Comparison against base commit a3daa08.

@jukzi
Copy link
Contributor Author

jukzi commented Jun 9, 2023

ignoring random fail #95

@jukzi jukzi merged commit 1b277d5 into eclipse-platform:master Jun 9, 2023
@jukzi jukzi deleted the Mismatched branch June 9, 2023 07:11
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.

3 participants