-
Notifications
You must be signed in to change notification settings - Fork 1
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
Composer #3
base: master
Are you sure you want to change the base?
Conversation
…d/installed dependencies users can now specify everything* they need in composer.json
- Sorry but it got messy. - Default level changed to 5 - Added composer 'properly'
PR Changes:
Notes:
Basic tests passed
TODO:
|
2 years. This pr is still alone. |
@NhanAZ how is this actually useful to most people anyway? It is just for phpstan analysis, and it's easier to do that on a github action. |
This might be viable again since new virion spec 3.0 where virions are via composer.json too this solves the analysis problem of shaded virions. Only remaining issue is PHP binary used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the PHP version, I think the easiest thing would be to build and tag different images for PM4
and PM5
, and then select the appropriate tag based on the API version declared in plugin.yml
.
@@ -1,21 +1,41 @@ | |||
FROM pmmp/pocketmine-mp:latest | |||
FROM ubuntu:18.04 as phpBuild |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
18.04 is obsolete
RUN bash compile.sh -t linux64 -f -u -g -l -j | ||
|
||
# New slate to lose all unwanted libs (~300mb lost here) | ||
FROM ubuntu:18.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same again here
RUN mkdir /php | ||
RUN apt-get update && apt-get install --no-install-recommends -y ca-certificates wget make autoconf automake libtool-bin m4 gzip bzip2 bison g++ git cmake pkg-config re2c | ||
WORKDIR /php | ||
RUN wget -q https://raw.githubusercontent.com/pmmp/php-build-scripts/stable/compile.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work for PM5.
Is this likely to be merged any time soon, or should I continue working with the existing image? |
RUN apt-get update && apt-get install --no-install-recommends -y ca-certificates wget make autoconf automake libtool-bin m4 gzip bzip2 bison g++ git cmake pkg-config re2c | ||
WORKDIR /php | ||
RUN wget -q https://raw.githubusercontent.com/pmmp/php-build-scripts/stable/compile.sh | ||
RUN bash compile.sh -t linux64 -f -u -g -l -j |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These flags will need updating - -u
is no longer used and -l
now has a different purpose
also, -j
expects a value for number of compile threads
Ideally I want this to be the way forward, much cleaner just to use composer but it’s a very outdated PR Hopefully I can update my local poggit once PM5 things are updated and continue continue some poggit duct tape |
Also need to see if this is viable on the server, since it will probably result in higher resource usage per build |
Build rate is not very high, so server resources are sufficient. But I would still advise using GitHub actions for more flexibility. |
IMO this is missing the point. Poggit is still going to exist for a while to come and the system should work properly while people continue to use it. I use GitHub Actions for all my plugins, but they still have to be published by Poggit. |
I mean you could just disable the Poggit lint if you already use GitHub CI actively. |
I would imagine it's helpful to reviewers to have a quick overview of the plugin's quality, no? |
To be clear, the reason I started using Actions is because of Poggit CI's shortcomings, some of which this PR might resolve. |
Writeup needed.