Skip to content

Conversation

@jeffersoncasimir
Copy link
Contributor

@jeffersoncasimir jeffersoncasimir commented Nov 8, 2022

Brief summary of changes

  • Modified the instructions to include the correct PHP library versions required for both Ubuntu and CentOS.
  • Added the missing variable assignment to the section titled: 'Creating the lorisadmin user'.
  • Changed an argument of the chown command in 'Creating the lorisadmin user' to prevent unwanted side-effects of creating a user with a period in its name.

Link(s) to related issue(s)

Resolves #8220

Copy link
Contributor

@christinerogers christinerogers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @jeffersoncasimir LGTM.
The missing variable assignment was originally intended to avoid picking the directory name for new projects, but it was definitely leading to some confusion based on comments heard over the years, so being more explicit should be helpful.

@driusan @ridz1208 any objections on specifying $projectname as loris as Jefferson has added here? It should reduce friction for new installs adn they can always change the name when they walk through these steps.

@kongtiaowang could you please pull and test, and review ? thanks

@christinerogers christinerogers added the Category: Documentation PR or issue that aims to improve the documentation label Nov 14, 2022
@driusan
Copy link
Collaborator

driusan commented Nov 17, 2022

I don't have any strong opinions on the variable but if we're going to hardcode the value we might as well get rid of the variable and just hardcode the path that uses it.

@driusan
Copy link
Collaborator

driusan commented Dec 13, 2022

@jeffersoncasimir ping.. are you going to get rid of the variable since it's hardcoded?

Copy link
Contributor Author

@jeffersoncasimir jeffersoncasimir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, makes sense.

@driusan driusan merged commit a310108 into aces:main Dec 13, 2022
@ridz1208 ridz1208 added this to the 25.0.0 milestone Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Category: Documentation PR or issue that aims to improve the documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[install] Update install docs - php 8 (loris main/25+)

5 participants