Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-130197: Remove obsolete pygettext tests#130198

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

StanFromIreland
Copy link
Contributor

@StanFromIrelandStanFromIreland commentedFeb 16, 2025•
edited by bedevere-appbot
Loading

@bedevere-appbedevere-appbot added testsTests in the Lib/test dir awaiting review labelsFeb 16, 2025
@bedevere-appbedevere-appbot mentioned this pull requestFeb 16, 2025
18 tasks
@StanFromIreland
Copy link
ContributorAuthor

Requesting@tomasr8

picnixz
picnixz previously requested changesFeb 16, 2025
Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

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

I don't think we need to remove those tests. They are testing that--docstring works from end-to-end. So I think we can keep them.

@bedevere-app

This comment was marked as resolved.

@StanFromIreland
Copy link
ContributorAuthor

@tomasr8 what was the plan for the future of it? Deprecate the option?

@picnixzpicnixz dismissed theirstale reviewFebruary 16, 2025 21:07

Considering the issue discussion, let's remove the tests while trying to have an E2E test for--docstring

Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

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

I'm no expert inpygettext, so I'll let@tomasr8 and@serhiy-storchaka decide. However, Serhiy wanted to keep all tests I think (see#130197 (comment)) so I think we can also close this PR but keep the issue to improve the test coverage instead (unless there's another issue for that)

@picnixzpicnixz added the pendingThe issue will be closed if no feedback is provided labelFeb 24, 2025
@tomasr8
Copy link
Member

Yes let's close this. I think Serhiy is right in the sense that even though the tests are currently not doing anything, if we ever change the implementation (though unlikely), they will be useful again to prevent potential regressions. Sorry@StanFromIreland for the extra work :/

I'll update the issue so we can reuse it for improving the test coverage, I think it's better than opening a new one.

@AA-TurnerAA-Turner removed the pendingThe issue will be closed if no feedback is provided labelApr 6, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@picnixzpicnixzpicnixz left review comments

@tomasr8tomasr8Awaiting requested review from tomasr8

Assignees
No one assigned
Labels
awaiting reviewskip newstestsTests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@StanFromIreland@tomasr8@picnixz@AA-Turner
[8]ページ先頭
©2009-2025 Movatter.jp