Bugfix #3943 - Clean template fails to show @uses if no description is provided in DocBlock #3944
Conversation
* Updated twig template to properly render reference name for @uses declerations ** Tested and working for method.html.twig but other two have been untested/unverifie
Dockerfile
Outdated
|
|
||
| COPY . /opt/phpdoc | ||
| # This should speed up the build process faster when developing | ||
| RUN mkdir -p /opt/phpdoc |
There was a problem hiding this comment.
Because of the change on the next line this folder doesn't exist
Dockerfile
Outdated
| COPY . /opt/phpdoc | ||
| # This should speed up the build process faster when developing | ||
| RUN mkdir -p /opt/phpdoc | ||
| COPY ./composer* /opt/phpdoc |
There was a problem hiding this comment.
Since we're installing composer dependencies at this time we only need the composer files not the source
There was a problem hiding this comment.
This could be tricky, there are a number of composer plugins active that need more files. So changing this could lead to unexpected behavior in the future during development.
There was a problem hiding this comment.
Good point, I'll see if there's another way I can provide the same end result without creating conflicts. Maybe with a new image target?
Dockerfile
Outdated
|
|
||
| FROM dev as dev-pcov | ||
| # Move the full copy last so modifications don't triger a re-install | ||
| COPY . /opt/phpdoc |
There was a problem hiding this comment.
Copy the code over LAST so that we can use the docker cache as much as possible for faster builds
There was a problem hiding this comment.
Not sure we have this, but the vendor directory should be in the dockerignore. Otherwise a possible local vendor dir would be built in.
There was a problem hiding this comment.
These changes are untested.
There was a problem hiding this comment.
These changes are untested.
| <dd><a href="{{ tag.reference|route('url') }}"><span class="namespace-wrapper">{{ tag.description ?: tag.reference }}</span></a></dd> | ||
| <dd> | ||
| <span class="namespace-wrapper"> | ||
| <a href="{{ tag.reference|route('url') }}">{{ tag.reference }}</a> |
There was a problem hiding this comment.
Changed the layout since it's logical for the reference to be clickable and the comment to not be.
There was a problem hiding this comment.
These changes are untested.
| @@ -1,18 +1,22 @@ | |||
| version: '3.8' | |||
There was a problem hiding this comment.
To fix the attribute version is obsolete, it will be ignored, please remove it to avoid potential confusion warning message
docker-compose.yml
Outdated
| volumes: | ||
| - ./:/opt/phpdoc | ||
| # Use the container vendor folder not the one from ./ | ||
| - /opt/phpdoc/vendor |
There was a problem hiding this comment.
Without this we are mapping the current directory to the container and so the vendor folder is missed, with this line docker now knows to ignore that container when mapping and use the existing folder contents from our early build stage.
docker-compose.yml
Outdated
| volumes: | ||
| - ./:/opt/phpdoc | ||
| # Use the container vendor folder not the one from ./ | ||
| - /opt/phpdoc/vendor |
There was a problem hiding this comment.
Without this we are mapping the current directory to the container and so the vendor folder is missed, with this line docker now knows to ignore that container when mapping and use the existing folder contents from our early build stage.
| context: . | ||
| target: dev-pcov | ||
| dockerfile: Dockerfile | ||
| user: ${CURRENT_UID} |
There was a problem hiding this comment.
Not used anywhere.
| context: . | ||
| target: dev | ||
| dockerfile: Dockerfile | ||
| user: ${CURRENT_UID} |
There was a problem hiding this comment.
Not used anywhere.
There was a problem hiding this comment.
This is being used in the makefile. Which I use for development. so please keep this
There was a problem hiding this comment.
I will add this back with a comment.
jaapio
left a comment
There was a problem hiding this comment.
This is the first round of review comments. I haven't tested anything yet. I will do this later
|
I've removed the Docker commit's from this PR. |
|
Enabled auto merge. It might be merged when the pipeline passes. An trigger an auto build for docker. So you can use v3-unstable to check your changes after everything is done. Thanks for your contribution! |
|
You're welcome @jaapio! Thanks for the quick turn around here. |
See: #3943 (comment)