Skip to content

Clean-up + Refactorings#129

Open
AHReccese wants to merge 9 commits intodevfrom
refactorings
Open

Clean-up + Refactorings#129
AHReccese wants to merge 9 commits intodevfrom
refactorings

Conversation

@AHReccese
Copy link
Copy Markdown
Member

@AHReccese AHReccese commented Apr 21, 2026

Reference Issues/PRs

What does this implement/fix? Explain your changes.

In this PR, I apply internal clean-up and refactoring to better cover corner cases, improve robustness.

  • Token validation at construction so callers cannot reach the network with empty/invalid credentials.
  • Regression tests covering token validation and CWD preservation.
  • upload now builds the template package in an isolated temporary directory and restores CWD + cleans up via finally.
  • Credentials are passed to twine via subprocess env= instead of mutating os.environ
  • pre-existing TWINE_* are stripped to prevent external overrides.
  • First subprocess failure is reported as-is (no longer overwritten by next commands).
  • Template generators updated to reflect reserver's real Python support (>=3.7, 3.7–3.14).
  • Default package metadata in params.py reworked to stop referencing unrelated third-party URLs/emails.
  • Test names shortened and explanations moved to inline comments.

Any other comments?

@AHReccese AHReccese added this to the reserver 0.8 milestone Apr 21, 2026
@AHReccese AHReccese self-assigned this Apr 21, 2026
@AHReccese AHReccese added bug Something isn't working enhancement New feature or request refactoring labels Apr 21, 2026
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Apr 21, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results
Complexity 0
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes. Give us feedback

…vironment state and asserting it remains unchanged after upload attempt.
…ad safety and prevent unintended overrides. The new implementation inherits non-TWINE variables and injects reserver's credentials without mutating os.environ.
@AHReccese AHReccese marked this pull request as ready for review April 22, 2026 22:33
@AHReccese
Copy link
Copy Markdown
Member Author

Hi Sadra @sadrasabouri,
I tried to add proper comments to explain the changes in the code, feel free to ask any questions you might have.

Best,
AmirH

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 80.43478% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.66%. Comparing base (d71d7d3) to head (aada650).
⚠️ Report is 142 commits behind head on dev.

Files with missing lines Patch % Lines
reserver/uploader.py 79.55% 4 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #129      +/-   ##
==========================================
- Coverage   82.76%   77.66%   -5.10%     
==========================================
  Files           5        6       +1     
  Lines          87      179      +92     
  Branches       10       31      +21     
==========================================
+ Hits           72      139      +67     
- Misses          6       24      +18     
- Partials        9       16       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Copy link
Copy Markdown
Member

@sadrasabouri sadrasabouri 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 overall. I only have a minor suggestion.

Comment thread reserver/params.py
Comment on lines -11 to +15
"author_email": "test@test.com",
"url": "https://url.com",
"download_url": "https://download_url.com",
"source": "https://github.com/source",
"author_email": "reserver@openscilab.com",
"url": RESERVER_REPO_URL,
"download_url": RESERVER_REPO_URL,
"source": RESERVER_REPO_URL,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it actually better to have reserved information for the reserved package? I don't think so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants