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.

Sign up for GitHub

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

Jump to bottom

Add support for installer #44

Merged
merged 1 commit into from
Apr 26, 2023
Merged

Add support for installer #44

merged 1 commit into from
Apr 26, 2023

Conversation

neidiom
Copy link
Contributor

@neidiom neidiom commented Apr 21, 2023

fixes #31

@neidiom
Copy link
Contributor Author

neidiom commented Apr 21, 2023

@t2d @doobry-systemli any luck?

@t2d
Copy link
Contributor

t2d commented Apr 24, 2023

I'm not super happy with the way variables are handled. We do have save defaults, which is good. On the other hand, these defaults will delete the installer in the first run and any change to them afterwards will be meaningless. I think, at least, we would have to add is some very clear instructions in the README. Do you agree?
Or we could think about a smarter way and maybe even merge the installer variables into one?

@neidiom
Copy link
Contributor Author

neidiom commented Apr 25, 2023

@t2d please review.

@t2d
Copy link
Contributor

t2d commented Apr 25, 2023

Thanks. Looks better already. What do you think about also running Unarchive roundcube when roundcube_enable_installer?
I think we have to assume that it's either the first run and then the unarchive will run anyway or the installer directory was deleted before and we have to re-extract it. Do you agree?

@neidiom neidiom closed this Apr 26, 2023
@neidiom neidiom reopened this Apr 26, 2023
@neidiom
Copy link
Contributor Author

neidiom commented Apr 26, 2023

@t2d please review :)

@t2d t2d merged commit f323752 into systemli:main Apr 26, 2023
@t2d
Copy link
Contributor

t2d commented Apr 26, 2023

Thanks. Lets see how this behaves in deployment and maybe we can even fix #29 with this new variable?

@0x46616c6b 0x46616c6b mentioned this pull request May 12, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Should the role support enable_installer in config.php?
2 participants
@neidiom @t2d