You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "ppkarwasz (via GitHub)" <gi...@apache.org> on 2025/03/27 08:02:56 UTC
[PR] Lock Antora dependencies [logging-parent]
ppkarwasz opened a new pull request, #367:
URL: https://github.com/apache/logging-parent/pull/367
We use [`npm-shrinkwrap.json`](https://docs.npmjs.com/cli/v11/configuring-npm/npm-shrinkwrap-json) to publish a locked list of dependency versions.
Other repositories will use this file to download a locked list of dependencies.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Lock Antora dependencies [logging-parent]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #367:
URL: https://github.com/apache/logging-parent/pull/367#discussion_r2096313003
##########
pom.xml:
##########
@@ -1576,6 +1575,10 @@
<goal>npm</goal>
</goals>
<phase>pre-site</phase>
+ <configuration>
+ <arguments>install github:apache/logging-parent#feature/366-npm-shrinkwrap</arguments>
+ <!--<arguments>install github:apache/logging-parent#rel/${stable-project.version}</arguments>-->
Review Comment:
Fixed in https://github.com/apache/logging-parent/pull/367/commits/5cb465e7dfa9cff6cad5d55ace0a775c8ee75eef
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Lock Antora dependencies [logging-parent]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #367:
URL: https://github.com/apache/logging-parent/pull/367#issuecomment-2784482041
@ppkarwasz, mind sharing the motivation for this feature, please? I mean, what do we fix? If we don't fix anything, but improve something, what is that we improve and why do we want this improvement?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Lock Antora dependencies [logging-parent]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #367:
URL: https://github.com/apache/logging-parent/pull/367#discussion_r2096312765
##########
.github/workflows/deploy-site-reusable.yaml:
##########
@@ -115,6 +105,12 @@ jobs:
git config user.name "ASF Logging Services RM"
git config user.email private@logging.apache.org
+ # Checking out a new branch will delete the `node_modules` folder
+ - name: Save Node.js modules
+ shell: bash
+ run: |
+ zip -qX "$RUNNER_TEMP"/node_modules.zip node_modules
Review Comment:
Fixed in https://github.com/apache/logging-parent/pull/367/commits/c5229479cbe1b1332f088d0eff4c481582103723
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Lock Antora dependencies [logging-parent]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #367:
URL: https://github.com/apache/logging-parent/pull/367#discussion_r2072430211
##########
.github/workflows/deploy-site-reusable.yaml:
##########
@@ -115,6 +105,12 @@ jobs:
git config user.name "ASF Logging Services RM"
git config user.email private@logging.apache.org
+ # Checking out a new branch will delete the `node_modules` folder
+ - name: Save Node.js modules
+ shell: bash
+ run: |
+ zip -qX "$RUNNER_TEMP"/node_modules.zip node_modules
Review Comment:
Shall we add `-0` to speed things up a bit?
##########
pom.xml:
##########
@@ -1576,6 +1575,10 @@
<goal>npm</goal>
</goals>
<phase>pre-site</phase>
+ <configuration>
+ <arguments>install github:apache/logging-parent#feature/366-npm-shrinkwrap</arguments>
+ <!--<arguments>install github:apache/logging-parent#rel/${stable-project.version}</arguments>-->
Review Comment:
I presume this will be fixed before merging this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Lock Antora dependencies [logging-parent]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz merged PR #367:
URL: https://github.com/apache/logging-parent/pull/367
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Lock Antora dependencies [logging-parent]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #367:
URL: https://github.com/apache/logging-parent/pull/367#issuecomment-2804855550
I modified the way `node_modules` are cached: since running a second `actions/checkout` in the same workflow deletes `node_modules`, the folder must be restored at the end of the workflow.
> [!CAUTION]
> Remove the temporary entries marked with `FIXME` from `deploy-site.yaml` before merging and remove the `test-site-stg-out` branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Lock Antora dependencies [logging-parent]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #367:
URL: https://github.com/apache/logging-parent/pull/367#issuecomment-2799011452
> > transitive NPM dependencies will be upgraded only by Dependabot
>
> Does this mean Dependabot will try to upgrade hundreds of dependencies listed in `npm-shrinkwrap.json`?
Yes, Dependabot will upgrade about a hundred NPM dependencies, but only once a month using a single PR.
See https://github.com/apache/logging-parent/pull/367/commits/c1134f89b82bebdf40d2f90bd2f939083d0ef244.
> > This is also suggested in the deploy-site-reusable workflow
>
> Doesn't this warrant a correction on `deploy-site-reusable.yaml` in this PR?
I modified the key used for the Antora cache, so that only one cache per `logging-parent` version is used. We don't need more caches, since `node_modules` should only contain JavaScript files and the `node` folder does not need to be cached (Node is stored in the Maven local repository).
See https://github.com/apache/logging-parent/pull/367/commits/b1f15b10d8a6d23949667c5b4028caa93b18fa12
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Lock Antora dependencies [logging-parent]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #367:
URL: https://github.com/apache/logging-parent/pull/367#discussion_r2043956202
##########
github.json:
##########
Review Comment:
What is this and why is this needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Lock Antora dependencies [logging-parent]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #367:
URL: https://github.com/apache/logging-parent/pull/367#discussion_r2044218634
##########
github.json:
##########
Review Comment:
Nice catch! :100:
This was a temporary file with `${{ github.toString() }}` I created to see if the branch name or commit SHA1 of the reusable workflow is stored somewhere. I planned to use it as the cache key for the Node.js dependencies.
Fixed in https://github.com/apache/logging-parent/pull/367/commits/52d9b02839900d7f09a255880095a79d69be0a68
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Lock Antora dependencies [logging-parent]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #367:
URL: https://github.com/apache/logging-parent/pull/367#issuecomment-2784663961
There are several advantages of locking the versions of the Antora dependencies:
1. The first one is certainly security: transitive NPM dependencies will be upgraded only by Dependabot, when we release a new version.
2. We can also save time on caching, since we can reuse an NPM workflow. This is also suggested in the `deploy-site-reusable` workflow:
https://github.com/apache/logging-parent/blob/9206282b1aa4b58a100662a80f7f5c3526f35137/.github/workflows/deploy-site-reusable.yaml#L81-L95
**Note**: I think that there is currently a bug in the workflow since `${{ hashFiles('node', 'node_modules') }}` is used **before** those folders are created in the `mvn site` step.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Lock Antora dependencies [logging-parent]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #367:
URL: https://github.com/apache/logging-parent/pull/367#issuecomment-2785429750
> transitive NPM dependencies will be upgraded only by Dependabot
Does this mean Dependabot will try to upgrade hundreds of dependencies listed in `npm-shrinkwrap.json`?
> This is also suggested in the deploy-site-reusable workflow
Doesn't this warrant a correction on `deploy-site-reusable.yaml` in this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org