Skip to content

fix bug in Timestamp.add #403

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

Merged
merged 2 commits into from
May 12, 2025
Merged

Conversation

ajtmccarty
Copy link
Contributor

instantiate the new timestamp as self.__class__ instead of Timestamp so that we keep the correct class

for example, infrahub has its own Timestamp class that inherites from the sdk's Timestamp and calling .add on an infrahub Timestamp yields an sdk Timestamp

Copy link

codecov bot commented May 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

@@            Coverage Diff             @@
##           stable     #403      +/-   ##
==========================================
+ Coverage   73.80%   75.12%   +1.32%     
==========================================
  Files          92       93       +1     
  Lines        8548     8560      +12     
  Branches     1680     1678       -2     
==========================================
+ Hits         6309     6431     +122     
+ Misses       1793     1676     -117     
- Partials      446      453       +7     
Flag Coverage Δ
integration-tests 34.42% <0.00%> (+9.02%) ⬆️
python-3.10 47.50% <100.00%> (+0.20%) ⬆️
python-3.11 47.50% <100.00%> (+0.20%) ⬆️
python-3.12 47.50% <100.00%> (+0.22%) ⬆️
python-3.13 47.52% <100.00%> (+0.22%) ⬆️
python-3.9 45.79% <100.00%> (+0.11%) ⬆️
python-filler-3.12 24.26% <0.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/timestamp.py 82.35% <100.00%> (ø)

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@dgarros dgarros left a comment

Choose a reason for hiding this comment

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

Looks good,
I think we should do the same change for subtract as well just below add

Copy link

cloudflare-workers-and-pages bot commented May 12, 2025

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: d762fb6
Status: ✅  Deploy successful!
Preview URL: https://e3c2d131.infrahub-sdk-python.pages.dev
Branch Preview URL: https://ajtm-05092025-timestamp-add.infrahub-sdk-python.pages.dev

View logs

@ajtmccarty ajtmccarty merged commit 5d99664 into stable May 12, 2025
18 checks passed
@ajtmccarty ajtmccarty deleted the ajtm-05092025-timestamp-add-fix branch May 12, 2025 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants