Skip to content

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

Fix compile error caused by HDDS-9816 and HDDS-9303.

https://issues.apache.org/jira/browse/HDDS-9816

How was this patch tested?

Compiled locally.

Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix @adoroszlai

Copy link
Contributor

@sadanand48 sadanand48 left a comment

Choose a reason for hiding this comment

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

Thanks for reporting & fixing it, @adoroszlai , @sodonnel .

@adoroszlai adoroszlai merged commit 582a5ce into apache:master Dec 11, 2023
@adoroszlai adoroszlai deleted the HDDS-9816-addendum branch December 11, 2023 13:29
@adoroszlai
Copy link
Contributor Author

Thanks @hemantk-12, @sadanand48, @sodonnel for the review.

@adoroszlai
Copy link
Contributor Author

this change caused compile failure

@ChenSammi it fixed the compile error that was caused by the change mentioned in PR description.

Your PR is still built against the previous commit:

HEAD is now at ecb77b7 Merge 32e628e into fd9bb1e

https://github.com/apache/ozone/actions/runs/7168115694/job/19515934863?pr=5749#step:2:491

To fix it, please merge current master into your branch.

@ChenSammi
Copy link
Contributor

ChenSammi commented Dec 12, 2023

this change caused compile failure

@ChenSammi it fixed the compile error that was caused by the change mentioned in PR description.

Your PR is still built against the previous commit:

HEAD is now at ecb77b7 Merge 32e628e into fd9bb1e

https://github.com/apache/ozone/actions/runs/7168115694/job/19515934863?pr=5749#step:2:491

To fix it, please merge current master into your branch.

Right, @adoroszlai , this patch fixes the compile issue. Just curious, the JIRAs caused compile failure are all recent merged. How does their CI build not find the compile failure?

@hemantk-12
Copy link
Contributor

this change caused compile failure

@ChenSammi it fixed the compile error that was caused by the change mentioned in PR description.
Your PR is still built against the previous commit:

HEAD is now at ecb77b7 Merge 32e628e into fd9bb1e

https://github.com/apache/ozone/actions/runs/7168115694/job/19515934863?pr=5749#step:2:491
To fix it, please merge current master into your branch.

Right, @adoroszlai , this patch fixes the compile issue. Just curious, the JIRAs caused compile failure are all recent merged. How does their CI build not find the compile failure?

I think both commits were fine independently and CI passed on both of them. Compilation failed when both were merged into master branch.

@adoroszlai
Copy link
Contributor Author

How does their CI build not find the compile failure?

Events:

Git reports conflict for the second PR if commits on master edit the same lines, but it doesn't know about code syntax/semantics.

If I remember correctly there is an option in Github to only allow PR merge if its branch is up-to-date with the base branch. But that requires more work for both developers and CI.

The current behavior is similar to optimistic locking in DBs. I think it's fine in most cases.

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.

5 participants