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

Need to deal with collisions when renaming and merging namespaces #717

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

amvanbaren
Copy link
Contributor

Fixes #716
Merge namespace members
Return error if namespaces contain duplicate extensions

Merge namespace members
Return error if namespaces contain duplicate extensions
@amvanbaren amvanbaren self-assigned this Mar 23, 2023
@kineticsquid
Copy link
Contributor

kineticsquid commented Mar 27, 2023

@amvanbaren
Testing:

  • renamed namespace to one that exists w/out merge
    • Correctly failed
  • renamed namespace to one that exists w merge and duplicate extension
    • Correctly failed
  • renamed namespace to one that exists w same member in same role in both
    • Correctly resulted in member listed once
  • added myself (kineticsquid) as an owner to sasjs along with amvanbaren. Then attempted to merge namespace sasjs into kineticsquid (in which kineticsquid is only a member).
    • This test appeared to fail; both namespaces exist as they had prior to my rename/merge request. The rename/merge request appeared to have no effect.

On perhaps import ant detail: after submitting the final test, I got distracted on another topic before coming back to this task. I then could not remember if I'd actually submitted the namespace rename request (since after the pop up message you're left on the same page). So I submitted the request again. Not sure if this duplicate request caused any problems. Could you check the logs?

@kineticsquid
Copy link
Contributor

@amvanbaren Follow up comment. I made the above comment about 20 min after I did the final test (given the pop up message). On a whim I went back about an hour later and noticed that the test had completed successfully. It could have taken anywhere from about 25 min to 60 min to complete. Is that expected? Should we update the information in the popup message?

@amvanbaren
Copy link
Contributor Author

@amvanbaren Follow up comment. I made the above comment about 20 min after I did the final test (given the pop up message). On a whim I went back about an hour later and noticed that the test had completed successfully.

You mean this test?

added myself (kineticsquid) as an owner to sasjs along with amvanbaren. Then attempted to merge namespace sasjs into kineticsquid (in which kineticsquid is only a member).

It could have taken anywhere from about 25 min to 60 min to complete. Is that expected?

Yes, the popup says: "15 minutes to a couple hours"

@kineticsquid
Copy link
Contributor

Doh... should have read that more thoroughly. Thanks! We're good.

@amvanbaren amvanbaren merged commit cdf9e01 into eclipse:master Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Need to deal with collisions when renaming and merging namespaces
2 participants