Skip to content
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

Add with_suffix #670

Merged
merged 8 commits into from
Feb 14, 2022
Merged

Add with_suffix #670

merged 8 commits into from
Feb 14, 2022

Conversation

q0w
Copy link
Contributor

@q0w q0w commented Jan 21, 2022

What do these changes do?

Adds suffix, suffixes, with_suffix

Are there changes in behavior for the user?

Related issue number

Closes #613

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 Jan 21, 2022
@codecov
Copy link

codecov bot commented Jan 23, 2022

Codecov Report

Merging #670 (69f5791) into master (97dce30) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #670   +/-   ##
=======================================
  Coverage   99.72%   99.73%           
=======================================
  Files           4        4           
  Lines         739      750   +11     
  Branches      168      170    +2     
=======================================
+ Hits          737      748   +11     
  Misses          2        2           
Flag Coverage Δ
unit 99.60% <100.00%> (+0.14%) ⬆️

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

Impacted Files Coverage Δ
yarl/_url.py 99.65% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97dce30...69f5791. Read the comment docs.

yarl/_url.py Outdated Show resolved Hide resolved
Copy link
Member

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

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

Please add an explicit test that replaces one suffix with other.

@q0w
Copy link
Contributor Author

q0w commented Jan 24, 2022

@asvetlov done

@q0w q0w requested a review from asvetlov February 1, 2022 12:35
@asvetlov asvetlov merged commit 6b29670 into aio-libs:master Feb 14, 2022
@asvetlov
Copy link
Member

Thanks!

q0w added a commit to q0w/yarl that referenced this pull request Mar 29, 2022
@michaelaye
Copy link

Hi! I got v1.7.2 and this merge is still missing from it, it seems?

@q0w
Copy link
Contributor Author

q0w commented Oct 20, 2022

@michaelaye I think, it exists in v1.8

@michaelaye
Copy link

ah, but no conda package after 1.7.2 hence the confusion. Is there a problem with building conda? Need help?

@q0w
Copy link
Contributor Author

q0w commented Oct 20, 2022

conda-forge/yarl-feedstock#45 is still open

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.

Add with_suffix() method
3 participants