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

Move 'engines_submit_last' from config system to state system #83

Open
jenlampton opened this issue Apr 5, 2019 · 4 comments

Comments

@jenlampton
Copy link
Member

commented Apr 5, 2019

Every time I do a config sync, I show my xmlsitemap config file has changed. Usually, this is because the engines_submit_last value has been updated. This should probably be stored in state, instead of config, to prevent this problem.

@opi

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2019

Same as backdrop-contrib/backup_migrate#43 , so the corresponding PR could help finding inspiration : https://github.com/backdrop-contrib/backup_migrate/pull/44/files

Steps:

  • Use state_set()/state_get() instead of config_set()/config_get()
  • Implements a hook_update_N to get config value (config_get()) and save in state (state_set())
  • Remove existing key from config with config_clear()
@herbdool

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2019

I took a look at this. Seems that there are other variables that should also be in the state system and removed from config: generated_last, regenerate_needed, rebuild_needed. It also means xmlsitemap_var should probably be removed since it can't be used for both state and config, and the todo on the function mentions it's not needed.

@herbdool

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2019

A PR #85. Please do some manually testing. Not all the tests are passing but I'm not sure if they were passing before. That could use work regardless to ensure the changes here don't break anything.

@herbdool

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2019

I ran the tests without this PR and many of the same things are failing, so this PR doesn't make it any worse at least.

@laryn laryn added the has pr label Aug 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.