feat: add fortnightly interval #1955
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JP-Ellis
force-pushed
the
feat/fortnight-interval
branch
4 times, most recently
from
6fba497 to
4a34f47
Compare
yagebu
requested changes
Feb 8, 2025
| @@ -93,6 +96,31 @@ class Interval(Enum): | |||
| YEAR = "year" | |||
| QUARTER = "quarter" | |||
| MONTH = "month" | |||
| FORTNIGHT = "fortnight" | |||
| # Note: While a fortnight is a well-defined time interval, there are no | |||
There was a problem hiding this comment.
Can you move this (maybe in a slightly shortened form) to the help pages this seems relevant for users to understand how this works
In some countries (such as Australia), it is very common for wages to be paid on a fortnightly basis, that is, every 2 weeks. As a result, a number of other expenses are also paid on a fortnightly basis. This commit extends the `Interval` enum by adding a `FORTNIGHT` option. I have added a test for that, and have updated the translations where possible (not that other than for English and French, the other translations may be suboptimal). Resolves: beancount#1939 Signed-off-by: JP-Ellis <[email protected]>
Made a few improvements to the budgets docs by: - Specifying how budgets can replace previous one using an example - Expanding a bit on how each interval is calculated - Some minor refactors to help everything flow a bit better Signed-off-by: JP-Ellis <[email protected]>
JP-Ellis
force-pushed
the
feat/fortnight-interval
branch
from
4a34f47 to
84fcdbb
Compare
I also realised I misunderstood the 'get_prev_interval' which returns that start of the current interval. So good to have improved test coverage. Signed-off-by: JP-Ellis <[email protected]>
|
I have updated the tests to ensure 100% test coverage. I did have to add to lines with For the coverage, would you be open to a PR adding integration with Codecov? This will help see coverage of PRs automatically, and it is free for OSS projects. You can see an example of how Codecov integrates within GitHub at: pact-foundation/pact-python#935 And you can follow links therein to see the more detailed coverage reports. |
|
Any further changes needed before this can be merged? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In some countries (such as Australia), it is very common for wages to be paid on a fortnightly basis, that is, every 2 weeks. As a result, a number of other expenses are also paid on a fortnightly basis.
This commit extends the
Intervalenum by adding aFORTNIGHToption. I have added a test for that, and have updated the translations where possible (not that other than for English and French, the other translations may be suboptimal).As part of this commit, I have also taken the opportunity to fix a confusion between ISO weeks and calendar weeks as the two do not always match, andEDIT: I have moved this into #1956.2025-W01is the ISO format indicating the week starting on December 30 2024. Let me know if you want me to split out this particular change into a separate PR.Resolves: #1939