Replace '-w' Perl flag with use warnings;

[rhansen]

Now the shebang line conforms with Debian policy >= 4.1.2 (see https://www.debian.org/doc/debian-policy/ch-files.html#scripts).

Also delete the useless 2nd shebang line.

[rhansen]

Approved but not merged? I'm confused...

[SuperSandro2000]

I don't feel comfortable merging this on my own cause and I don't want to break master for other people. So waiting for the other, sadly a bit inactive, maintainers.

The same applies for your other PRs. Sorry about the delays.

[DaveSophoServices]

Sandro, first, thank you for being active here. I had really high hopes of being useful, but my work had needs, now I’m busy looking after 5 kids 10 and under. I’m still here and plan to re-involve when I can. :-) I believe this change shouldn’t break anything, when was this “use warnings” introduced in Perl 5? I think it’s standard in all our use base? Dave

…

On May 25, 2020, at 9:45 PM, Sandro ***@***.***> wrote: I don't feel comfortable merging this on my own cause and I don't want to break master for other people. So waiting for the other, sadly a bit inactive, maintainers. The same applies for your other PRs. Sorry about the delays. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[rhansen]

when was this “use warnings” introduced in Perl 5? I think it’s standard in all our use base?

It was introduced in Perl 5.6, which was released 20 years ago (March 2000).

I don't feel comfortable merging this on my own cause and I don't want to break master for other people.

Understood—it's definitely a good thing if everyone can rely on master working well. It's tricky to find the right balance between accepting change and providing reliability. Some things to keep in mind:

• Changes can always be reverted.
• As you already know, reviews are important. At least one reviewer (not the author) should look at every change before it is merged. Reviews won't catch everything, but they do prevent most bugs.
• Adding a second (or third) reviewer is useful for complex changes or if the first reviewer is not familiar with the code. Otherwise, a single reviewer is usually sufficient.
• A comprehensive test suite provides confidence that a change will not break anything. That confidence greatly accelerates development. (Unfortunately, I don't see any tests in the ddclient codebase.)
• The health of the contributor community is more important than the health of the code on master:
  • With many active contributors, bugs will be quickly found and fixed.
  • Taking too long to merge pull requests will discourage future contributions.
  • Requiring perfection from contributors will discourage future contributions.
  • If too many pull requests pile up, there's a risk that someone will get frustrated and fork the project, and that will discourage future contributions.
• Working closely with a distribution (e.g., Debian) is a good way to get users to quickly test new changes. (I'm currently working on reviving active maintenance of the Debian ddclient package.)
• It's important to communicate expectations to prospective developers (code style, allowed dependencies, backwards compatibility requirements, tests, code review procedure, etc.).

[rhansen]

I bumped the minimum required Perl version to v5.8.0 for reasons explained in the commit message.

[SuperSandro2000]

Working closely with a distribution (e.g., Debian) is a good way to get users to quickly test new changes. (I'm currently working on reviving active maintenance of the Debian ddclient package.)

Good to know that. The current version on Debian is a bit old and I would really appreciate it if would be updated.

So I am just going ahead and merging this so that we get it up to standards for the Debian package.

[rhansen]

It looks like the commit was squashed before merging, and that discarded the commit message. I think the commit message is important to keep in the Git history so that people know the why, not just the what.

Would you mind force-updating master to the commit in this PR?

git clone git@github.com:ddclient/ddclient.git &&
cd ddclient &&
git fetch https://github.com/rhansen/ddclient.git shebang &&
git reset --hard FETCH_HEAD &&
git push -f

[wimpunk]

We should also check the default perl version used by other distributions. I remember I had to revert a commit because I wrongly assumed all distributions had that version. Unfortunatly I don't remember which version bump we tried then.

[rhansen]

As a matter of project policy, how far back do you want to support? If support for older Perl versions is dropped too soon then users will have trouble running ddclient. If support is kept too long then it becomes difficult for developers to add new features or clean up cruft.

I think that dropping support for Perl < 5.8.0 (released 2002) is reasonable. Someone who hasn't upgraded their Perl since 2002 is unlikely to upgrade their ddclient so they'll never notice (though this is less true for router firmware).

I personally would like to see support kept for at least the version of Perl in Debian oldstable and in the oldest supported Ubuntu release (ignoring ESM). This makes it easier to backport a new ddclient package to those old releases. Debian oldstable currently has Perl 5.24.1 and Ubuntu 16.04 has Perl 5.22.1.

[SuperSandro2000]

I just force pushed master with non squashed PRs. This is the first and last time we do this because it is basically against every good practise.

Sorry if I mess up forks or clones.

[SuperSandro2000]

I personally would like to see support kept for at least the version of Perl in Debian oldstable and in the oldest supported Ubuntu release (ignoring ESM). This makes it easier to backport a new ddclient package to those old releases. Debian oldstable currently has Perl 5.24.1 and Ubuntu 16.04 has Perl 5.22.1.

Reasonable idea which I support.

We probably should not discuss this on a closed merge request and open an issue about this if we need to talk about it more.

[rhansen]

I just force pushed master with non squashed PRs.

Thank you! I rebased all of my PRs against current master.

We probably should not discuss this on a closed merge request and open an issue about this if we need to talk about it more.

Sounds good; I'll open an issue.