? Pending

User tests: Successful: Unsuccessful:

avatar SharkyKZ
SharkyKZ
16 Oct 2020

Summary of Changes

Updates static HTML pages after changes in #31007.

Testing Instructions

Run node build.js --build-pages. Check if Git tracks any changes.

Actual result BEFORE applying this Pull Request

These files keep getting modified:

templates/system/incompatible.html
templates/system/build_incomplete.html

Expected result AFTER applying this Pull Request

No more files are getting modified.

Documentation Changes Required

No.

avatar SharkyKZ SharkyKZ - open - 16 Oct 2020
avatar SharkyKZ SharkyKZ - change - 16 Oct 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 16 Oct 2020
Category Front End Templates (site)
avatar SharkyKZ SharkyKZ - change - 16 Oct 2020
The description was changed
avatar SharkyKZ SharkyKZ - edited - 16 Oct 2020
avatar SharkyKZ
SharkyKZ - comment - 16 Oct 2020

And do we actually need to keep these files in the repository?

avatar richard67
richard67 - comment - 16 Oct 2020

And do we actually need to keep these files in the repository?

I'd say no, but maybe I'm missing something.

@wilsonge Do you know why we have templates/system/incompatible.html and templates/system/build_incomplete.html in the sources when we rebuild them anyway?

avatar richard67 richard67 - test_item - 16 Oct 2020 - Tested successfully
avatar richard67
richard67 - comment - 16 Oct 2020

I have tested this item successfully on b267e56


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31108.

avatar dgrammatiko
dgrammatiko - comment - 16 Oct 2020

And do we actually need to keep these files in the repository?

Yes they're used both in the installation and the index.php

if (version_compare(PHP_VERSION, JOOMLA_MINIMUM_PHP, '<'))
{
die(
str_replace(
array('{{PHP_VERSION}}', '{{BASEPATH}}'),
array(JOOMLA_MINIMUM_PHP, 'http://' . $_SERVER['SERVER_NAME'] . '/'),
file_get_contents(dirname(__FILE__) . '/../templates/system/incompatible.html')
)
);
}

joomla-cms/index.php

Lines 18 to 27 in 5813af5

if (version_compare(PHP_VERSION, JOOMLA_MINIMUM_PHP, '<'))
{
die(
str_replace(
'{{PHP_VERSION}}',
JOOMLA_MINIMUM_PHP,
file_get_contents(dirname(__FILE__) . '/templates/system/incompatible.html')
)
);
}

avatar richard67
richard67 - comment - 16 Oct 2020

Yes they're used both in the installation and the index.php

But they are generated by the build script, or not? That should be sufficient for these cases, or am I missing something?

avatar dgrammatiko
dgrammatiko - comment - 16 Oct 2020

But they are generated by the build script, or not?

Unless I messed up really bad, it should after running this command:

* node build.js --build-pages === will create the error pages (for incomplete repo build PHP+NPM)

avatar richard67
richard67 - comment - 16 Oct 2020

Unless I messed up really bad, it should after running this command:

* node build.js --build-pages === will create the error pages (for incomplete repo build PHP+NPM)

Yes, and that's the subject of this PR and I have tested it.

So why should we keep the result pages in the GitHub sources?

avatar dgrammatiko
dgrammatiko - comment - 16 Oct 2020

So why should we keep the result pages in the GitHub sources?

It's the same reason the project still keeps all the generated css in the templates, etc... (iirc at some point, before getting auto generated installables per PR, there was a decision to keep these files in place as a fallback). They shouldn't exist in the repo, if you ask me...

avatar richard67
richard67 - comment - 16 Oct 2020

They shouldn't exist in the repo, if you ask me...

Then it seems we agree.

avatar SharkyKZ
SharkyKZ - comment - 16 Oct 2020

Forget my question ?‍♂️. We do need these. build_incomplete.html is shown to users running on Git clones but before they run any build scripts.

avatar richard67
richard67 - comment - 16 Oct 2020

Forget my question ?‍♂️. We do need these. build_incomplete.html is shown to users running on Git clones but before they run any build scripts.

Ahhhh, silly me, sure I should have known that. That's the purpose of that file.

avatar ceford
ceford - comment - 16 Oct 2020

Not sure about this: I noticed that both were marked as changed after I did a Refresh - so they were not there at some time. Deleted them and ran the build - file are there. Committed so the change marker vanished. Ran build again - no change, Repeated without the patch - no change.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31108.

avatar wilsonge wilsonge - close - 16 Oct 2020
avatar wilsonge wilsonge - merge - 16 Oct 2020
avatar wilsonge wilsonge - change - 16 Oct 2020
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-10-16 23:08:36
Closed_By wilsonge
Labels Added: ?
avatar wilsonge
wilsonge - comment - 16 Oct 2020

We're all good - think we're all back on the same page. LGTM. Thanks!

Add a Comment

Login with GitHub to post a comment