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 composer autoloader #1544

Merged
merged 1 commit into from
Feb 20, 2023
Merged

Add composer autoloader #1544

merged 1 commit into from
Feb 20, 2023

Conversation

miaulalala
Copy link
Contributor

@miaulalala miaulalala commented Feb 9, 2023 •
edited
Loading

Fixes #1535

Signed-off-by: Anna Larch anna@nextcloud.com

@ChristophWurst I added the autoloader class according to the docs. Is there anything else I need to do?

@miaulalala miaulalala added 3. to review Waiting for reviews performances Performances issues and optimisations labels Feb 9, 2023
@miaulalala miaulalala self-assigned this Feb 9, 2023
@miaulalala miaulalala mentioned this pull request Feb 9, 2023
Closed
@ChristophWurst
Copy link
Member

@ChristophWurst I added the autoloader class according to the docs. Is there anything else I need to do?

This is a shipped app, so there are no build steps allowed. You will have to vendor the vendor/composer/* and vendor/autoload.php` files. Otherwise the new file points to a file that does not exist and no class can be loaded in the released version of this app.

ChristophWurst
ChristophWurst requested changes Feb 9, 2023
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

#1544 (comment)

@ChristophWurst
Copy link
Member

See also the additional context in #1535.

@ChristophWurst
Copy link
Member

Looks good. Did you force add the files? Adjust .gitignore to no longer ignore the added vendor/autoload.php and vendor/composer/*

@miaulalala
Copy link
Contributor Author

It's in a new composer folder, not in the vendor dir on suggestion by @juliushaertl

ChristophWurst
ChristophWurst approved these changes Feb 10, 2023
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Looks good

@juliusknorr
Copy link
Member

php-cs-fixer should ignore the composer directory.

@miaulalala miaulalala force-pushed the enh/add-autoloader-classes branch from 0bfd733 to f7ac6eb Compare February 13, 2023 11:32
@miaulalala
Add composer autoloader
495060e 
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala miaulalala force-pushed the enh/add-autoloader-classes branch from 480967d to 495060e Compare February 20, 2023 10:22
@miaulalala miaulalala enabled auto-merge February 20, 2023 10:22
@miaulalala miaulalala merged commit cfe0b49 into master Feb 20, 2023
@miaulalala miaulalala deleted the enh/add-autoloader-classes branch February 20, 2023 10:29
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliusknorr juliusknorr juliusknorr approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

Assignees

@miaulalala miaulalala

Labels
3. to review Waiting for reviews performances Performances issues and optimisations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Perf: generate and ship authoritative Composer class loader
3 participants
@miaulalala @ChristophWurst @juliusknorr