Update the links pointing to php.net

[avpaderno]

The links pointing to php.net used in comments and in the user interface should be changed to:

Links without sub-domain (http://php.net)

• Use https://
• Use www as sub-domain
• Don't have any language selector

Links with sub-domain (for example, http://bugs.php.net)

• Use https://

Removing the language selector in the link allows users to see the page in the language they selected. Providing the language selector isn't necessary as, for example, http://www.php.net/manual/en/function.rawurlencode.php#86506 and http://www.php.net/manual/fr/function.rawurlencode.php#86506 will both show the same comment.

[avpaderno]

I would like to open a PR for this.

[avpaderno]

Just a follow-up: I am going to reinstall Linux on my computer in the next days. I will open a PR after I am done with this.

[klonos]

Thanks for raising this @kiamlaluno 👍🏼 ...yay for consistency! 🙂

[avpaderno]

Finally, I found back this issue. Now that I set up my new computer, I can work on this, but that means tomorrow, or between four days. If somebody wants to work on this, feel free to create a PR. (I will do my best to work on it.)

[klonos]

Meh, there's plenty of other issues in the queue that we can work on @kiamlaluno, and a day or a few days won't save the world ...this one can be yours 😉

[avpaderno]

There is an error in the tests I cannot understand.

Error: Command failed: phpcs --report=json -q --encoding=undefined --standard=../phpcs/Backdrop --runtime-set ignore_errors_on_exit 1 --runtime-set ignore_warnings_on_exit 1 core/includes/bootstrap.inc core/includes/common.inc core/includes/database/database.inc core/includes/database/log.inc core/includes/database/mysql/schema.inc core/includes/errors.inc core/includes/file.inc core/includes/filetransfer/filetransfer.inc core/includes/stream_wrappers.inc core/includes/unicode.inc core/includes/uuid.inc core/modules/date/date.admin.inc core/modules/date/views/date_views_argument_handler_simple.inc core/modules/date/views/date_views_filter_handler_simple.inc core/modules/dblog/dblog.module core/modules/file/file.install core/modules/image/image.install core/modules/link/link.module core/modules/simpletest/backdrop_web_test_case.php core/modules/simpletest/simpletest.install core/modules/simpletest/tests/ajax.test core/modules/simpletest/tests/common.test core/modules/simpletest/tests/file.test core/modules/simpletest/tests/system_test.module core/modules/syslog/syslog.module core/modules/system/image.gd.inc core/modules/system/system.admin.inc core/modules/system/system.archiver.inc core/modules/system/system.install core/modules/system/system.mail.inc core/modules/system/system.module core/modules/system/system.tar.inc settings.php

[indigoxela]

@kiamlaluno my guess: somewhere in one of all the changed files there's a PHP syntax error?

Odd, by the way: you made it the WorldWideWebWhat? (4 w everywhere :laughing:)

[avpaderno]

I will start again with a fresh fork.

[avpaderno]

I checked the local copy with phpcs, but it only reported issues for the settings.php file. I checked all the .php, .inc, .install, .module, and .test files with php -l, but it didn't report any syntax error.

[indigoxela]

Very strange - it just failed again... I'll give it a try locally. :thinking:

[avpaderno]

I also used phpcs:ignore and phpcs:disable to ignore the commented out code, but phpcs still fails. I guess it must not be related with code issues, or phpcs would give a more informative error message.

[indigoxela]

I have no idea, what's going on. Nothing obvious. And locally phpcs runs just fine (the exact same phpcs version as in the test run). It might be a problem with the action or with the runner. :shrug:

However, other PR did run just fine... :thinking:

[avpaderno]

I only committed the changes to the URLs, for now.

[indigoxela]

I only committed the changes to the URLs, for now.

Yes, all totally harmless changes - and phpcs still chokes. I don't get it. :face_with_spiral_eyes:

The change in system.tar.inc should get reverted, though, as this is actually not our code. The code comments explain why.

[avpaderno]

I reverted those changes. It makes sense, as they make more difficult to port the file if there is the need.

[indigoxela]

Sooo... I'm in the process to investigate what's going on, why phpcs tries to "pester" your PR and non of the others.

In fact, there seem to be standard violations in your changed lines. The big question is, why the (external) action fails to create annotations for that...

Worst case scenario - the action is broken. https://github.com/thenabeel/action-phpcs

But let's try to fix coding standard in changes. Many thanks for your patience @kiamlaluno. :pray:

[avpaderno]

phpcs reports also missing type hinting. It takes time to fix those, as that requires reading the documentation and the code. It took time to fix everything reported by phpcs for a single file, and I am not sure I chose the perfect hinting. Probably a different person would choose a different hint.
For example, for a string parameter that by default gets NULL as value, is it better string|null or simply string? Should stClass be used, or object?

[indigoxela]

@kiamlaluno again, many thanks for your patience. I belief, you haven't joined the Backdrop community, yet, when the decision for this GitHub Action (thenabeel/action-phpcs) was made, so I'll try to summarize:

Backdrop has loads of standard violations, as it has loads of legacy code from Drupal. Fixing all at once isn't possible - there are many topics with higher priority. However, we wanted newly added (or edited) code to conform - and that's what that action provides. We already discovered that there are circumstances, where this action also causes false positives (code that isn't changed in the current PR, but has been provided by the same person in an earlier PR). In this case it's optional to fix it, but also OK to not fix it, in case the PR provider thinks it would complicate a later use of git blame.

So:

1. It's not necessary to fix all violations in a file, it's just requested that new code conforms.
2. If previously touched code - not part of the current PR - shows up annotated, it's up to the PR providers if they fix that or not. Core committers will accept.

What now seems to happen in your PR is that the action seems a bit flaky. Not sure if we can fix that. But what I'd like to know first is - does the action behave again, when the violations in currently changed code get fixed, or do we have to seek for an alternative action.

[indigoxela]

Seems like we're stuck with the phpcs problem - which only seems to affect this PR, but none of the others. I'll open an extra issue for that.

Edit: #5893