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

Redo "Revert "Deprecate String>>asDate" #15232

Conversation

guillep
Copy link
Member

@guillep guillep commented Nov 4, 2023

Redo #15152, originally reverted because there are still broken tests.

Date>>asDate is ambiguous regarding how to interpret the day, month and year fields. Sometimes it interprets them in some order and sometimes in other.

'1-13-04' asDate => 13 january 2004
'13-1-04' asDate => 13 january 2004

Recently an exception was added in one of the cases above, but the ambiguity remains.
We propose with @guillep to deprecate it in favor of #readFrom:pattern: with an explicit pattern.

We deprecate methods:

  • String>>asDate,
  • Date>>fromString, and
  • Date>>readFrom.
    Remove related tests on the deprecated methods.
    Use Date>>#readFrom:pattern: specifying a concrete pattern instead + all other.

We also added an option into the pattern parser to continue parsing using the continueAfterParsing method.

@jecisc jecisc added the Status: Need more work The issue is nearly ready. Waiting some last bits. label Nov 4, 2023
@@ -1,7 +1,7 @@
Extension { #name : 'Project' }

{ #category : '*Calypso-SystemQueries' }
Project class >> convertToCalypsoBrowserItem: aProject [
Copy link
Member Author

Choose a reason for hiding this comment

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

I got this change because of a merge. @jecisc do you know why?

Copy link
Member

Choose a reason for hiding this comment

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

I have no idea where those "classSide" are coming from.. I've see it in the past in some projects but I never discovered why.

I never do my merges with Pharo, maybe it's the Pharo merger that uses another printer?

@guillep
Copy link
Member Author

guillep commented Nov 9, 2023

Strange, I have lots of tests failing that look unrelated to my changes, and look more related to the handling of protocols and extension methods, any idea?

imagen

@guillep
Copy link
Member Author

guillep commented Nov 9, 2023

They work in my local image based on build 1020.

Pharo 12.0.0
Build information: Pharo-12.0.0+build.1020.sha.373a7ce1fe48d19844129ab9086329fcf758ce50 (64 Bit)

@jecisc
Copy link
Member

jecisc commented Nov 9, 2023

This is a known failure I'm working on that appears really recently: #15282

The error is really strange because it happens when updating the users of a trait, but the trait is generated in the tests and has no user (:

I'll try to fix it this week

@guillep
Copy link
Member Author

guillep commented Nov 9, 2023

Oh! Not my fault! Then I can remove the WIP from this PR :)

@guillep guillep changed the title [WIP] Redo "Revert "Deprecate String>>asDate" Redo "Revert "Deprecate String>>asDate" Nov 9, 2023
@Ducasse Ducasse closed this Dec 8, 2023
@Ducasse Ducasse deleted the revert-15231-revert-15228-revert-15227-revert-15152-clean/dates branch December 8, 2023 13:23
@guillep
Copy link
Member Author

guillep commented Dec 8, 2023

Why closing it instead of merging it?

@jecisc jecisc restored the revert-15231-revert-15228-revert-15227-revert-15152-clean/dates branch December 8, 2023 13:46
@jecisc jecisc reopened this Dec 8, 2023
…t-15228-revert-15227-revert-15152-clean/dates
…1-revert-15228-revert-15227-revert-15152-clean/dates
@guillep guillep merged commit edb7a1a into Pharo12 Feb 14, 2024
1 of 2 checks passed
@guillep guillep deleted the revert-15231-revert-15228-revert-15227-revert-15152-clean/dates branch February 14, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Need more work The issue is nearly ready. Waiting some last bits.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants