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

[Backport]3437-Adding-an-instance-class-variable-remove-the-traits-of-a-class #3438

Conversation

@jecisc
Copy link
Member

commented May 29, 2019

Backport fix in class slot addition.

Fixes #3437

@jecisc jecisc changed the base branch from Pharo8.0 to Pharo7.0 May 29, 2019

@MarcusDenker

This comment has been minimized.

Copy link
Member

commented May 29, 2019

you need to look at fillFor:, too... it was missing setting something in Pharo8

@jecisc

This comment has been minimized.

Copy link
Member Author

commented May 29, 2019

Oh, ok. Just this already fix the problem we had, but maybe there is another one?

@MarcusDenker

This comment has been minimized.

Copy link
Member

commented May 29, 2019

ah, the fix there might have been a side effect of removing copyClassSlotsFromExistingClass...

@jecisc

This comment has been minimized.

Copy link
Member Author

commented May 29, 2019

So, what would be the clean solution? Do you have an idea?

@jecisc

This comment has been minimized.

Copy link
Member Author

commented May 30, 2019

I reverted the change on fillFor: since it can cause problem.

@MarcusDenker
Copy link
Member

left a comment

Ok

@jecisc

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2019

Faling tests are already failing for me in a clean image.

@MarcusDenker MarcusDenker requested a review from estebanlm Jun 3, 2019

@MarcusDenker

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

So do we merge this? It is for Pharo7

@jecisc

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

@tesonep Is that Ok for you?

@MarcusDenker

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

So what do we do?

@MarcusDenker

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

If nobody objects, this will be merged end of the week.

@jecisc

This comment has been minimized.

Copy link
Member Author

commented Jun 25, 2019

We can ask people to review during the sprint

@jecisc jecisc closed this Jun 27, 2019

@jecisc jecisc deleted the jecisc:3437-Adding-an-instance-class-variable-remove-the-traits-of-a-class branch Jun 27, 2019

@jecisc jecisc restored the jecisc:3437-Adding-an-instance-class-variable-remove-the-traits-of-a-class branch Jul 1, 2019

@jecisc jecisc reopened this Jul 1, 2019

@jecisc

This comment has been minimized.

Copy link
Member Author

commented Jul 1, 2019

Oups, I closed it by accident because I cleaned my branched and this one had more than 1 month.

@MarcusDenker MarcusDenker merged commit 7a80338 into pharo-project:Pharo7.0 Jul 15, 2019

2 of 3 checks passed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details
WIP Ready for review
Details
probot/minimum-reviews No pending reviews
@MarcusDenker

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

I finally merged it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.