[BF] Add EOF newlines

[rlaager]

As you have seen, I've been submitting patches to IXP-Manager. One small thing has been making that annoying. So I'm submitting a PR to get your thoughts on this.

EOF newlines are conventional on Unix. Not having them means that every time I edit a file (wherein my editor adds the missing newline), I end up with a diff that I need to either: A) manually strip out before committing, or B) ship in the commit cluttering it up and making the project slightly inconsistent.

Conforming to the EOF newline convention fixes this.

The second commit was generated as follows:

find . -not -wholename "./.git/**" -not -name "*.svg" -type f -exec grep -Iq . {} \; -print | while read inputfile ; do if [ -s "$inputfile" ] && [ "$(tail -c1 "$inputfile"; echo x)" != $'\nx' ]; then echo "" >>"$inputfile" ; fi ; done

Credit to https://stackoverflow.com/a/13659891 and https://stackoverflow.com/a/10082466 for that. I certainly could have come up with something from scratch, but through the magic of stackoverflow, I didn't have to! 😀

Note that I'm ignoring .svg files. I don't feel strongly about those either way. While they are actually text, I'm happy to treat them as "binary" for these purposes, as nobody hand edits them.

[barryo]

As you have seen, I've been submitting patches to IXP-Manager.

Yes, thank you!

One small thing has been making that annoying. So I'm submitting a PR to get your thoughts on this.

Trailing blank lines is just an editor setting. I use PHPStorm and I don't ever recall making a change to the default setting. I use the same PHPStorm across multiple projects so I'm afraid the end result will be trading your convenience for mine!

Ultimately, I'm not going to merge a commit that touches 800 files, sorry! Even if the change appears innocuous enough, the scope is far too wide.

We're looking to hire a full time dev on IXP Manager and I think the best thing to do will be for me to have this conversation with whomever that is as they'll be the one doing the heavy lifting.

[rlaager]

Trailing blank lines is just an editor setting. I use PHPStorm

https://stackoverflow.com/questions/36019534/add-an-empty-line-at-end-of-file-according-to-psr-2-on-phpstorm

We're looking to hire a full time dev on IXP Manager and I think the best thing to do will be for me to have this conversation with whomever that is as they'll be the one doing the heavy lifting.

That sounds like a good way forward. You might want to have that as a somewhat broader conversation about coding standards/styles. I, like everyone, have an opinion on coding styles. But these days, I generally yield my personal thoughts to language/framework standards, as consistency across projects is usually more important than whatever my pet thing is. (And to be clear, that's genuinely a statement about myself, not some backhanded reference to you.) I do mostly Python these days, so that would be e.g. PEP-8 there. I honestly don't know what the conventions are in the modern PHP space. That link above mentioned PSR-2, which seems to be superseded by PSR-12: https://www.php-fig.org/psr/psr-12/

Enforcing the coding standards with a linter, e.g. as part of the CI pipeline, is the next step. The gold standard (which I'll admit I haven't personally adopted much) is having the formatting be applied automatically (e.g. black for Python, clang-format for C) so people don't even have to think of it.

Anyway, maybe that helps you!

[barryo]

If you're in search of a standard(*), PSR-12 is the generally accepted standard these days, having extended PSR-2, but IXP Manager as a project predates those by ~7 years. My own coding in PHP predates those by ~15 years and C by ~25+ years.

There is a standard in IXP Manager - my standard 😉 - see PHPStorm's rules for it here. I didn't conjure this up myself but rather it was learned from various sources way back when.

The general rule, like any open source project, is match the style in the project you're editing or the file you're editing if there's a doubt. Yann, the previous developer for ~5 years mostly matched me but we don't beat anyone into submission in exacting style. BTW, I don't consider end of file blanks part of my style - editors should just leave the file as is.

Looking forward - again, that'll be down to the future dev. Ultimately that person will be in the code full-time and he/she should have the latitude to make those calls on that basis. How they want to enforce it is a discussion we'll have with them.

We use PSR-12 in other projects and PHPStorm has linters built in; I have these enabled on some projects and not on others, depending on age generally. One issue in applying a different standard to an existing project is small commits become large commits where the change gets lost in the noise. Or files get rewritten to the extent that history and Git tools like blame because useless. It's a big decision with a wide blast radius.

---

Speaking generally and not about IXP Manager (I'm all in favour of PSR-12 for this project pending discussion with a future full-time dev; and I implement PSR-12 on other projects):

(*) I say "if you're in search of a standard" because there's no rule that a specific coding standard must be adhered to to program in PHP or Python or whatever. One of the ideas behind PSR-2/12 is to "reduce cognitive friction when scanning code from different authors". This is very laudable and certainly applies well to large OSS projects with lots of devs. If you're a developer on a project where 90+% of the code is yours, PSR-12 might cause cognitive friction. People's brains work differently; if you have a style that's embedded in your brain for decades, changing is hard. That's a long winded way of saying: programmers are generally aware of coding styles and linters, and if they're not applying what others may consider "the gold standard", it's probably because they're choosing a path of least/less resistance which may or does yield more productivity.

[rlaager]

It sounds like we agree 100% on general principles. :)