Skip to content

Deprecate Instant.period#1142

Merged
bonjourmauko merged 2 commits intomasterfrom
deprecate-instant-period
Dec 1, 2022
Merged

Deprecate Instant.period#1142
bonjourmauko merged 2 commits intomasterfrom
deprecate-instant-period

Conversation

@bonjourmauko
Copy link
Copy Markdown
Member

@bonjourmauko bonjourmauko commented Jul 31, 2022

Depends on #1160

Deprecations

  • In periods.Instant:
    • Remove period, method used to build a Period from an Instant.
    • This method created an upward circular dependency between Instant and Period causing lots of trouble.
    • The functionality is still provided by periods.period and the Period constructor.

Migration details

  • Replace some_period.start.period and similar methods with Period((unit, some_period.start, 1)).

@bonjourmauko bonjourmauko force-pushed the deprecate-instant-period branch from e15f4c7 to e26fb84 Compare July 31, 2022 19:24
@MattiSG
Copy link
Copy Markdown
Member

MattiSG commented Aug 1, 2022

I will postpone reviewing this PR until the one it depends on is approved, and suggest that this one is set to draft. Indeed, I fail to see how it could be approved when #1141 might not go through.

Do you truly need #1141 for this one to be reviewed? 🙂 As an alternative to switching to draft, I suggest to remove the dependency and rebase straight on master. Yes, test-docs might fail until #1141 is fixed, but that is true of all PRs, not only of this one. This would enable reviews to be parallelised and proper solutions found for #1140 without blocking all the others.

@bonjourmauko bonjourmauko force-pushed the deprecate-instant-period branch from e26fb84 to 42d592b Compare August 1, 2022 17:58
@bonjourmauko bonjourmauko force-pushed the deprecate-instant-period branch from 42d592b to ba418b8 Compare August 21, 2022 11:42
@bonjourmauko bonjourmauko added the kind:refactor Refactoring and code cleanup label Aug 21, 2022
@bonjourmauko bonjourmauko requested a review from a team August 21, 2022 11:42
@bonjourmauko
Copy link
Copy Markdown
Member Author

cc @MattiSG

@openfisca openfisca deleted a comment from coveralls Dec 1, 2022
@bonjourmauko bonjourmauko force-pushed the deprecate-instant-period branch from ba418b8 to 9c74d97 Compare December 1, 2022 16:54
@bonjourmauko bonjourmauko force-pushed the deprecate-instant-period branch from 9c74d97 to cb89a4b Compare December 1, 2022 17:12
@bonjourmauko bonjourmauko merged commit ba0afad into master Dec 1, 2022
@bonjourmauko bonjourmauko deleted the deprecate-instant-period branch December 1, 2022 17:15
@bonjourmauko bonjourmauko added this to the Add support for weeks milestone Sep 17, 2024
@bonjourmauko bonjourmauko mentioned this pull request Sep 17, 2024
@bonjourmauko bonjourmauko mentioned this pull request Sep 25, 2024
32 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:refactor Refactoring and code cleanup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants