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.

Sign up for GitHub

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

Jump to bottom

fix: #listAtCategoryNamed: has been deprecated in Pharo 12 #606

Merged
merged 7 commits into from
Sep 26, 2023
Merged

fix: #listAtCategoryNamed: has been deprecated in Pharo 12 #606

merged 7 commits into from
Sep 26, 2023

Conversation

theseion
Copy link
Collaborator

No description provided.

fniephaus
fniephaus approved these changes Sep 23, 2023
View reviewed changes
Copy link
Member

@fniephaus fniephaus left a comment

Choose a reason for hiding this comment

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

LGTM.

@theseion
Copy link
Collaborator Author

Unfortunately, the Pharo stable builds almost always segfault. I didn't manage to get a clean run of all builds.

@fniephaus
Copy link
Member

Pharo stable builds almost always segfault

Is this due to a new and fault VM? It's not related to your changes, right? Maybe we need to inform the VM maintainers and mark the test as allowed to fail?

@theseion
Copy link
Collaborator Author

I'm pretty sure it's not my fault. Since the Pharo-alpha builds are good, I suspect that the VM used for stable builds really has an issue that has been fixed upstream.

@fniephaus
Copy link
Member

smalltalkCI uses zeroconf scripts to pull the VM, so this is out of our control. Any suggestion how we could proceed?

@theseion
Copy link
Collaborator Author

It's weird. Looking at the builds, the one's for Pharo 11 (explicitly) are fine, the one's for "stable", which is Pharo 11, are not. But since we have both builds anyway, I suggest removing the "stable" builds.

Also, I'm not sure how much sense it makes to test newer versions against the 32-bit VM. We could say that we only support 64-bit explicitly as of Pharo 12 (usually, 32-bit will be fine). That will reduce the number of additional builds in the future

@theseion
Copy link
Collaborator Author

Actually, we could conceivably remove all the 32-bit builds. The only ones registered are for Pharo 3, stable and alpha.

@theseion
Copy link
Collaborator Author

I've committed a proposal to this PR.

fniephaus
fniephaus reviewed Sep 24, 2023
View reviewed changes
.github/workflows/main.yml Outdated
- Squeak32-trunk
- Squeak32-6.0
- Squeak32-4.5
- Pharo64-stable
Copy link
Member

Choose a reason for hiding this comment

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

Why remove Pharo64-stable here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. Because that's the failing build
  2. Because Pharo64-11 is a separate build, already in the list

I'm not saying this is a good suggestion :)

@fniephaus
Copy link
Member

Looks like you broke the GemStone jobs. Not sure what's going on and not got enough time to help. Sorry!

@fniephaus
Copy link
Member

Do we maybe need to upgrade the stable VMs?

smalltalkCI/pharo/run.sh

Line 182 in d3a45fe

echo "get.pharo.org/vm100"

@theseion
Copy link
Collaborator Author

It's the SentButNotImplementedTest from Rowan that fails. That's probably because of the Pharo only message that I'm using here:

smalltalkCI/repository/SmalltalkCI-Tests.package/SCIMonticelloLoadSpecTest.class/instance/testAddLoadedClassesFrom.st

Line 12 in 12a3bd3

packageName: 'SmalltalkCI'
.
Not sure what to do there, except for using dynamic sends to hide the selector...

@fniephaus
Copy link
Member

I can't remember how this works. Maybe @dalehenrich can help?

@theseion
Copy link
Collaborator Author

Do we maybe need to upgrade the stable VMs?

Oh! Yes, absolutely! I hand't realised that the VMs are hard coded. I'll do that.

@theseion
Copy link
Collaborator Author

I can't remember how this works. Maybe @dalehenrich can help?

It's not a GemStone issue. The issue is that the SmalltalkCI-Tests package is loaded for all platforms, but now Pharo is starting to diverge. So maybe we need to start moving those tests to platform specific packages... I'll try to come up with something.

@theseion theseion mentioned this pull request Sep 24, 2023
Closed
@theseion
Copy link
Collaborator Author

There's a hierarchy for that test now. What do you think?

@fniephaus
Copy link
Member

Yes, LGTM, thank you! When can this be merged? There is still a job failure.

@theseion
Copy link
Collaborator Author

#607 needs to be merged first.

@fniephaus
Copy link
Member

Can you cherry-pick the changes so we see a green gate?

@theseion
feat: update Pharo VM and image urls
9d366fc 
Note: This commit also changes `Pharo-stable` and `Pharo-alpha` to be
synonyms of `Pharo64-stable` and `Pharo64-alpha`, respectively.
Previously, these aliases referred to the 32-bit versions.
@theseion
chore: update url tests for Pharo
78b6bd2
@theseion
Copy link
Collaborator Author

Done.

fniephaus
fniephaus approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@fniephaus fniephaus left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@fniephaus fniephaus merged commit 7376bb8 into hpi-swa:master Sep 26, 2023
79 checks passed
@theseion theseion deleted the fix-pharo12-deprecation branch September 26, 2023 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fniephaus fniephaus fniephaus approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@theseion @fniephaus