Skip to content

Allow null in renv.lock lists#803

Merged
jonkeane merged 2 commits into
mainfrom
renv_cleanup
Jun 26, 2026
Merged

Allow null in renv.lock lists#803
jonkeane merged 2 commits into
mainfrom
renv_cleanup

Conversation

@jonkeane

Copy link
Copy Markdown
Collaborator

Intent

Allow lists to have null in them without breaking.

Resolves: #801

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change

Approach

Strip nulls out of lists in renv.lock files

Automated Tests

Added a test.

Directions for Reviewers

Checklist

  • I have updated CHANGELOG.md to cover notable changes.
  • I have updated all related GitHub issues to reflect their current state.
  • I have run the rsconnect-python-tests-at-night workflow in Connect against this feature branch.

@jonkeane jonkeane requested review from amol- and joshyam-k June 26, 2026 19:12
@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-06-26 22:23 UTC

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
7452 6127 82% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
rsconnect/environment_r.py 93% 🟢
TOTAL 93% 🟢

updated for commit: f4721ac by action🐍

"Version": "1.0.0",
"Source": "Repository",
"Repository": "CRAN",
"Imports": ["R6", None],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not use Null to match the exact case that this is intended to fix?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

If I'm following the code correctly, this is actually all a Python dict representation that will then get written to JSON at

json.dump(lockfile, f)
at that point it will be JSON null and then it will get read in and yadda yadda...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doh .py my bad!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's tricky cause they look sooooo close ... and then they are not 🫠

@joshyam-k joshyam-k left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

one comment otherwise 👍

@jonkeane jonkeane enabled auto-merge (squash) June 26, 2026 22:13
@jonkeane jonkeane merged commit 1753468 into main Jun 26, 2026
24 checks passed
@jonkeane jonkeane deleted the renv_cleanup branch June 26, 2026 22:23
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.

Cleanup renv.lock transformations

2 participants