Skip to content

Refine VCS URL install error message wording in Transaction.add_requirement - #278

Merged
ryanking13 merged 3 commits into
mainfrom
copilot/fix-error-message-formatting
May 21, 2026
Merged

Refine VCS URL install error message wording in Transaction.add_requirement#278
ryanking13 merged 3 commits into
mainfrom
copilot/fix-error-message-formatting

Conversation

Copilot AI commented May 20, 2026

Copy link
Copy Markdown
Contributor

Follow-up to PR #277: one review nit remained on the VCS-URL error text in Transaction.add_requirement. This updates the message formatting/wording to match the requested phrasing without changing behavior.

  • Error message wording (follow-up review nit)

    • Updated the ValueError guidance for VCS URLs in micropip/transaction.py:
      • from: “Provide a wheel URL or a package name available on a configured index.”
      • to: “Provide a URL pointing to a wheel or a package name available on a configured index.”
    • Kept existing detection logic (_looks_like_vcs_url) and control flow unchanged.
  • Test alignment

    • Updated the VCS URL error test in tests/test_transaction.py to assert the revised guidance sentence, so expected text stays synchronized with the user-facing error.
if isinstance(req, str) and _looks_like_vcs_url(req):
    raise ValueError(
        f"Cannot install {req!r}: micropip only installs prebuilt wheels "
        "and does not support installing from a VCS URL "
        "(git+, hg+, svn+, bzr+). Provide a URL pointing to a wheel "
        "or a package name available on a configured index."
    )
Original prompt

Create a follow-up pull request in pyodide/micropip that addresses the remaining review comments left on merged PR #277 ("Raise a helpful error for VCS URLs in install()").

Context:

  • PR Raise a helpful error for VCS URLs in install() #277 was merged into main on 2026-05-20.
  • The PR added a helpful ValueError for VCS URLs in Transaction.add_requirement, tests in tests/test_transaction.py, and a changelog entry.
  • After merge, a reviewer noted there was still one nit/comment worth applying in a follow-up PR.

Relevant review feedback to address:

  1. In micropip/transaction.py, reviewer agriyakhetarpal suggested reflowing the error message formatting for cleanliness. The intended updated code is:
        if isinstance(req, str) and _looks_like_vcs_url(req):
            raise ValueError(
                f"Cannot install {req!r}: micropip only installs prebuilt wheels "
                "and does not support installing from a VCS URL "
                "(git+, hg+, svn+, bzr+). Provide a URL pointing to a wheel "
                "or a package name available on a configured index."
            )

Notes:

  • The current merged code says: Provide a wheel URL or a package name available on a configured index.
  • The requested follow-up wording is: Provide a URL pointing to a wheel or a package name available on a configured index.
  • Preserve the surrounding logic and existing helper _looks_like_vcs_url unless additional cleanup is clearly beneficial.
  • Update or add tests only if needed to keep coverage correct.

Please create a PR with just the necessary changes to address that feedback cleanly and minimally.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: open a PR that addresses the comments in this pull request

Assistant: I’m waiting for your response to the confirmation dialog before I can continue with opening the follow-up PR.

This pull request was created from Copilot chat.

Copilot AI changed the title [WIP] Fix error message formatting for VCS URLs Refine VCS URL install error message wording in Transaction.add_requirement May 20, 2026
Copilot AI requested a review from ryanking13 May 20, 2026 08:40
@ryanking13

Copy link
Copy Markdown
Member

@copilot also update the PR number in the changelog

Copilot AI commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

@copilot also update the PR number in the changelog

Updated in commit 3c8073c: the changelog entry now references PR #277 instead of issue #77.

@agriyakhetarpal

Copy link
Copy Markdown
Member

I ran the CI

@agriyakhetarpal
agriyakhetarpal marked this pull request as ready for review May 20, 2026 13:25
@ryanking13
ryanking13 merged commit 00e7e6c into main May 21, 2026
8 checks passed
@agriyakhetarpal
agriyakhetarpal deleted the copilot/fix-error-message-formatting branch May 21, 2026 08:48
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.

3 participants