Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: support remote manifests #309

Merged
merged 6 commits into from
Sep 2, 2022
Merged

feat: support remote manifests #309

merged 6 commits into from
Sep 2, 2022

Conversation

WOSHIZHAZHA120
Copy link
Contributor

No description provided.

@innocenzi
Copy link
Owner

Can you add tests?

@WOSHIZHAZHA120
Copy link
Contributor Author

ok, i will try

@WOSHIZHAZHA120
Copy link
Contributor Author

Can you add tests?

finished, please review

@innocenzi innocenzi changed the title feat: try support cdn feat: support remote manifests Sep 2, 2022
@innocenzi innocenzi merged commit ec52102 into innocenzi:main Sep 2, 2022
@innocenzi
Copy link
Owner

Thanks! Please try not to update every changed file with your configured code style next time, though.

@WOSHIZHAZHA120
Copy link
Contributor Author

Sorry, I'm not familiar with php-cs-fixer, can you tell me which command should I use?

@WOSHIZHAZHA120
Copy link
Contributor Author

And did you forget to add a new config item in the config file?

@innocenzi
Copy link
Owner

Sorry, I'm not familiar with php-cs-fixer, can you tell me which command should I use?

php-cs-fixer has no rules for this, but you changed whitespaces in a few places (there and there for instance). No big deal but annoying for maintainers.

And did you forget to add a new config item in the config file?

No, the third parameter is the value returned if it doesn't exist in the config. I think this feature is quite niche, and since it isn't documented I didn't feel like it needed to have a default value in the configuration file. Feel free to PR though!

@WOSHIZHAZHA120
Copy link
Contributor Author

I think it would be better to change the config key?
after change the build_path, the build directory becomes /public/{url}, too long!

@innocenzi
Copy link
Owner

Feel free to PR whatever you need as long as it makes sense and you provide an explanation for the change 👍

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants