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

gh-57141: Add dircmp shallow option. #109499

Merged
merged 3 commits into from
Mar 4, 2024
Merged

Conversation

aunzat
Copy link
Contributor

@aunzat aunzat commented Sep 16, 2023

Add support for non shallow file comparison for filecmp.dircmp like there is for filecmp.cmp and filecmp.cmpfiles.

Based on (BPO) patch from @planet36. I have added unittests for the shallow options and
reorganized the tests somewhat. Maybe a "new in version" should be added to the documentation?

Co-authored-by: Steve Ward planet36@gmail.com

Issue: #57141


📚 Documentation preview 📚: https://cpython-previews--109499.org.readthedocs.build/

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Sep 16, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

Copy link
Member

@CuriousLearner CuriousLearner left a comment

Choose a reason for hiding this comment

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

Hi Tobias @aunzat 👋🏼

Thanks for your contributions!

Seems like the test case test_dircmp_shallow_same_file is failing. Can you please take a look?

I have a few suggestions mentioned inline.

Since this is your first contribution, if you prefer, you may add your name to Misc/ACKS file.

@aunzat
Copy link
Contributor Author

aunzat commented Sep 17, 2023

@CuriousLearner, thanks for comments

Seems like the test case test_dircmp_shallow_same_file is failing. Can you please take a look?

Fixed

I have a few suggestions mentioned inline.

Perfect, I have updated it accordingly.

@aunzat aunzat marked this pull request as draft September 25, 2023 16:18
@aunzat aunzat force-pushed the dircmp-non-shallow branch 2 times, most recently from e80c210 to 437b076 Compare September 26, 2023 05:02
Co-authored-by: Steve Ward <planet36@gmail.com>
Co-authored-by: Sanyam Khurana <8039608+CuriousLearner@users.noreply.github.com>
@aunzat aunzat marked this pull request as ready for review September 27, 2023 06:21
@encukou
Copy link
Member

encukou commented Mar 4, 2024

Maybe a "new in version" should be added to the documentation

For a new argument, there's versionchanged -- see the devguide.
I've uptated the branch, added the directive, and tested locally; since I had the commit I pushed it here directly rather than add a suggestion.

@encukou encukou enabled auto-merge (squash) March 4, 2024 16:48
@encukou encukou merged commit 60743a9 into python:main Mar 4, 2024
31 checks passed
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
Co-authored-by: Steve Ward <planet36@gmail.com>
Co-authored-by: Sanyam Khurana <8039608+CuriousLearner@users.noreply.github.com>
@aunzat aunzat deleted the dircmp-non-shallow branch March 4, 2024 19:17
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
Co-authored-by: Steve Ward <planet36@gmail.com>
Co-authored-by: Sanyam Khurana <8039608+CuriousLearner@users.noreply.github.com>
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
Co-authored-by: Steve Ward <planet36@gmail.com>
Co-authored-by: Sanyam Khurana <8039608+CuriousLearner@users.noreply.github.com>
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