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

Issue #70: update the README #71

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bschmalhofer
Copy link

@bschmalhofer bschmalhofer commented Sep 29, 2024

This PR is for #70.

I don't recall why I thought the Perl::Tidy was not included in the Docker image. Looking at the cpanfile I found that both Perl::Tidy and Perl::Criticare in the cpanfile.
So I only updated README.md to reflect the current state of affairs. There I assumed that Perl 5.10 is the earliest supported version of Perl.

into the block for Perl 5.12 or higher.
Actually it is not obvious why Pod::Readme would not work for Perl 5.10.
Assuming the Perl 5.10 is the earliest supported version.
Copy link
Collaborator

@oalders oalders left a comment

Choose a reason for hiding this comment

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

Thank you so much for this! I just had one comment.

- Test::Version
- Test::Warnings

## Only on Perl 5.10 and later
## Only on Perl 5.10
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we could clarify about these. We are pinning to specific versions on 5.10 as those should be the last 5.10 supported versions of those modules. They should also get installed in more recent Perls, but we haven't specified a version requirement. If they're not specifically listed elsewhere, I assume they are pulled in via dependencies or they were at the time when the change was made.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I thought the same. I'll verify that these are actually pulled in as dependencies and explicitly list them in the cpanfile. This means that fewer cases must be distinguished in Readme.mk.

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.

2 participants