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

Add new section on how to update locally #578

Merged
merged 14 commits into from Oct 30, 2020
Merged

Add new section on how to update locally #578

merged 14 commits into from Oct 30, 2020

Conversation

fiedsch
Copy link
Contributor

@fiedsch fiedsch commented Oct 8, 2020

This is a first draft for the new section (see #575). Comments and suggestions are welcome. Especially regarding the section "Ausblick" where I'm not sure if that makes sense, as a developer should know how to do that and it might look "frightening" to the non technical user. My current opinion: do not include it—the example is only half of the job anyway.

An english translation will follow once we agree on the german version.

@fiedsch fiedsch marked this pull request as draft October 8, 2020 10:55
@fkaminski
Copy link
Contributor

I would keep the paragraph "Ausblick" and possibly rename it to "Alternative Vorgehensweise über die Konsole" to make clear that this is just a different approach.
Maybe with a link to https://docs.contao.org/dev/reference/commands/ and then optionally with your further script examples for automation.

@fkaminski
Copy link
Contributor

fkaminski commented Oct 9, 2020

Possibly mention that this procedure is also possible/useful for new Contao installations (in a info box maybe).

@netzarbeiter netzarbeiter added the Two Month Review Docs in the Contao Two Month Review label Oct 14, 2020
Copy link
Contributor

@fritzmg fritzmg left a comment

Choose a reason for hiding this comment

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

Btw. what about doing it via the Contao Manager locally?

docs/manual/installation/update-contao.de.md Outdated Show resolved Hide resolved
Co-authored-by: Fritz Michael Gschwantner <fmg@inspiredminds.at>
@fiedsch
Copy link
Contributor Author

fiedsch commented Oct 20, 2020

Btw. what about doing it via the Contao Manager locally?

Why would I want to do this if I have the power of the command line ;-)

Serious reason for not recommending this:

The Contao Manager has enabled the resolver cloud by default. IMO this would generate a lot of unnecessary traffic for the Cloud Resolver (still people seem to do this as questions concerning the Manager where also XAMPP or MAMP are mentioned can be seen now and then).

Additionally: what would be the benefit compared to using the Manager on the web hosting?

@fritzmg
Copy link
Contributor

fritzmg commented Oct 20, 2020

Why would I want to do this if I have the power of the command line ;-)

Well this guide is targeted towards people who would usually use the Contao Manager and the Cloud Resolver, isn't it?

The Contao Manager has enabled the resolver cloud by default.

You need to disable that of course and mention that in the guide.

Additionally: what would be the benefit compared to using the Manager on the web hosting?

The benefit would be to resolve the dependencies locally, instead of using the Cloud Resolver.

@fiedsch
Copy link
Contributor Author

fiedsch commented Oct 20, 2020

I see. Having never used the manager locally I‘ll try this myself first and then update the PR.

Thanks for the feedback!

@fkaminski
Copy link
Contributor

I see. Having never used the manager locally I‘ll try this myself first and then update the PR.

Thanks for the feedback!

Möglichkeit Neu Installation über den CM:
Kopiere composer.json und composer.lock in das Hauptverzeichnis oberhalb von »web«.
Der CM erkennt dies und bietet eine Installation (via composer install) an

Möglichkeit bei bestehender Installation über den CM:
Kopiere composer.json und composer.lock in das Hauptverzeichnis oberhalb von «web«.
Aktiviere einen composer install im CM über »Systemwartung/Composer-Abhängigkeiten/Installer«.

@fkaminski
Copy link
Contributor

fkaminski commented Oct 21, 2020

The Contao Manager has enabled the resolver cloud by default.
You need to disable that of course and mention that in the guide.

I am not sure if this is necessary when using the .lock with composer install via the CM?

@fritzmg
Copy link
Contributor

fritzmg commented Oct 21, 2020

I am not sure if this is necessary when using the .lock with composer install via the CM?

This is a guide on how to resolve the dependencies locally.

@fkaminski
Copy link
Contributor

I am not sure if this is necessary when using the .lock with composer install via the CM?

This is a guide on how to resolve the dependencies locally.

You are absolutely right - Sorry ...

@fiedsch
Copy link
Contributor Author

fiedsch commented Oct 29, 2020

As suggested by @fritzmg I added a section on using the contao manager locally.

I additionally removed the section "Ausblick", as i rather see that as a new section in "Guides".

As I'm not a manager user, could you please check if the descriptions make sense? Thanks!

@fritzmg
Copy link
Contributor

fritzmg commented Oct 29, 2020

lgtm 👍 (disclaimer: I am not a CM user either).

Copy link
Member

@netzarbeiter netzarbeiter left a comment

Choose a reason for hiding this comment

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

Thank you, Andreas.

docs/manual/installation/update-contao.de.md Outdated Show resolved Hide resolved
docs/manual/installation/update-contao.de.md Outdated Show resolved Hide resolved
docs/manual/installation/update-contao.de.md Outdated Show resolved Hide resolved
docs/manual/installation/update-contao.de.md Outdated Show resolved Hide resolved
docs/manual/installation/update-contao.de.md Outdated Show resolved Hide resolved
docs/manual/installation/update-contao.de.md Outdated Show resolved Hide resolved
},
"require": {
...
```
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
```
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@netzarbeiter I do not get, what that change is supposed to correct.

The suggested change removes the closing ``` and inroduces a }. Do you want to add the } as the closing brace of "require": { block? The backticks to close the code block would still be required though.

Copy link
Member

Choose a reason for hiding this comment

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

I was trying to get rid of that.

github

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like the unclosed } of the pseudo json code caused the error

docs/manual/installation/update-contao.de.md Outdated Show resolved Hide resolved
docs/manual/installation/update-contao.de.md Outdated Show resolved Hide resolved
fiedsch and others added 9 commits October 30, 2020 15:42
Co-authored-by: netzarbeiter <netzarbeiter@users.noreply.github.com>
Co-authored-by: netzarbeiter <netzarbeiter@users.noreply.github.com>
Co-authored-by: netzarbeiter <netzarbeiter@users.noreply.github.com>
Co-authored-by: netzarbeiter <netzarbeiter@users.noreply.github.com>
Co-authored-by: netzarbeiter <netzarbeiter@users.noreply.github.com>
Co-authored-by: netzarbeiter <netzarbeiter@users.noreply.github.com>
Co-authored-by: netzarbeiter <netzarbeiter@users.noreply.github.com>
Co-authored-by: netzarbeiter <netzarbeiter@users.noreply.github.com>
adding a blank after the ` ```json` at least mademy editor's codehighlighting happy. Maybe it helps here too?
@netzarbeiter netzarbeiter marked this pull request as ready for review October 30, 2020 15:15
@netzarbeiter netzarbeiter merged commit 551b7a6 into contao:master Oct 30, 2020
@netzarbeiter netzarbeiter removed the Two Month Review Docs in the Contao Two Month Review label Oct 30, 2020
@fiedsch fiedsch deleted the local_composer_updates branch October 30, 2020 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants