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

Addendum to PR 2087 #2308

Merged
merged 12 commits into from
Jul 14, 2022
Merged

Addendum to PR 2087 #2308

merged 12 commits into from
Jul 14, 2022

Conversation

ADDISON74
Copy link
Contributor

@ADDISON74 ADDISON74 commented Jul 10, 2022

This PR adds the missing translation strings, removes word Magento, changes the label format with capitalize letters as it is throughout the Backend. It also adds a simple explicit comment.

This is how the Backend implementation looks like if this PR is merged:

screenshot

@github-actions github-actions bot added Component: Core Relates to Mage_Core translations Relates to app/locale labels Jul 10, 2022
@ADDISON74
Copy link
Contributor Author

ADDISON74 commented Jul 14, 2022

The default value used in OpenMage is "30 2 * * *" inherited from Magento. This value is displayed in the box but if the configuration is not saved it will not be recorded as a row in the core_config_data table.

Once it's saved in the table the default value will no longer be used, but the established one. If the box is left empty the cron will not run. If the value in the table is deleted the default value will be used again.

I made the necessary changes in the comment. I used the expression "no longer run" because until then the cron was running. The "never run" variant is not suitable because this cron can run in a certain situation.

PS - I answered @luigifab's question addressed in the initial PR. As long as the cron does not run if the box is empty, it is no longer necessary to mention in the comment the running on February 30th. I updated the screenshot too.

fballiano
fballiano previously approved these changes Jul 14, 2022
@justinbeaty
Copy link
Contributor

I would maybe suggest something like:

The default value is "30 2 * * *". Enter an empty value to disable automatic cache flushing.

But I think this line has seen many suggested revisions, so it doesn't matter too much in the end.

@ADDISON74
Copy link
Contributor Author

It should be fine now.

@elidrissidev elidrissidev merged commit 79ecbe8 into OpenMage:1.9.4.x Jul 14, 2022
@github-actions
Copy link
Contributor

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
7 runs  ±0  5 ✔️ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit 79ecbe8. ± Comparison against base commit e06fe2d.

@sreichel
Copy link
Contributor

Does translations still work with locale packs?

(IMHO only output should be changed)

@ADDISON74 ADDISON74 deleted the ADDISON74-cache-flush-cron-update branch July 16, 2022 07:44
@elidrissidev
Copy link
Member

@sreichel The translations modified in this PR were added by #2087 which didn't include translations.

elidrissidev pushed a commit to elidrissidev/magento-lts that referenced this pull request Jul 22, 2022
* Update system.xml

* Update Mage_Core.csv

* Update Mage_Core.csv

* Update system.xml

* Update system.xml

* Update Mage_Core.csv

* Update system.xml

* Update system.xml

* Update Mage_Core.csv

* Update system.xml

* Update Mage_Core.csv
@justinbeaty justinbeaty mentioned this pull request Aug 19, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Relates to Mage_Core translations Relates to app/locale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants