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

Ensure subMenu is opened (backport) #2376

Merged
merged 1 commit into from Feb 6, 2019

Conversation

Projects
None yet
4 participants
@StephanEggermont
Copy link
Contributor

StephanEggermont commented Jan 28, 2019

Solves Issue #2331, #2333. Backport for Pharo 7. See 22084 in Fogbugz

@estebanlm

This comment has been minimized.

Copy link
Member

estebanlm commented Jan 28, 2019

why this backport? why this is a critical fix?

@StephanEggermont

This comment has been minimized.

Copy link
Contributor Author

StephanEggermont commented Jan 28, 2019

It breaks client code

@estebanlm

This comment has been minimized.

Copy link
Member

estebanlm commented Jan 29, 2019

why? AFAIK, this never worked before (or it does not works since a lot of time).
and which client code?

@StephanEggermont

This comment has been minimized.

Copy link
Contributor Author

StephanEggermont commented Jan 29, 2019

It worked in Pharo 6. I have code that needs to work in Pharo 7 so I need the fix backported

@StephanEggermont

This comment has been minimized.

Copy link
Contributor Author

StephanEggermont commented Jan 31, 2019

unrelated failures

@estebanlm estebanlm requested a review from jecisc Feb 5, 2019

@estebanlm

This comment has been minimized.

Copy link
Member

estebanlm commented Feb 5, 2019

now that I remember, there was a reason why this was removed.
@jecisc you were working there, do you have an observation to do ?

@jecisc

jecisc approved these changes Feb 5, 2019

Copy link
Member

jecisc left a comment

The PR seems fine for me.

I introduce this problem while removing duplication. I forgot one parameter in this method.

@jecisc

This comment has been minimized.

Copy link
Member

jecisc commented Feb 5, 2019

Before each menu constructor was duplication the logic. I changed it to build in one method and have the other ones calling this one.

@MarcusDenker MarcusDenker merged commit 442fd72 into pharo-project:Pharo7.0 Feb 6, 2019

1 check failed

continuous-integration/jenkins/pr-merge This commit has test failures
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.