Skip to content

update_query({}) should be a no-op.#845

Merged
mjpieters merged 1 commit intomasterfrom
fix_update_query
Apr 19, 2023
Merged

update_query({}) should be a no-op.#845
mjpieters merged 1 commit intomasterfrom
fix_update_query

Conversation

@mjpieters
Copy link
Copy Markdown
Contributor

@mjpieters mjpieters commented Apr 19, 2023

What do these changes do?

The documentation states that url.update_query(None) clears the query string.

However, that means that other valid arguments, such as {}, should not.

Are there changes in behavior for the user?

This behaviour was recently fixed in #792, but this specific edgecase was then
introduced. We haven't released a version with that change included, however.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> (e.g. 588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the PR
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Apr 19, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 19, 2023

Codecov Report

Merging #845 (86197ce) into master (cc509d1) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #845   +/-   ##
=======================================
  Coverage   99.73%   99.73%           
=======================================
  Files           4        4           
  Lines         768      768           
  Branches      217      217           
=======================================
  Hits          766      766           
  Misses          2        2           
Flag Coverage Δ
unit 99.60% <100.00%> (ø)

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

Impacted Files Coverage Δ
yarl/_url.py 99.66% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mjpieters mjpieters merged commit 22df0bb into master Apr 19, 2023
@mjpieters mjpieters deleted the fix_update_query branch April 19, 2023 15:32
mjpieters added a commit that referenced this pull request Apr 19, 2023
mjpieters added a commit that referenced this pull request Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant