Fix build updateReadme inside zip package
#3065
Conversation
internal/builder/packages.go
Outdated
| return "", nil, fmt.Errorf("resolving transform manifests failed: %w", err) | ||
| } | ||
|
|
||
| updatedReadmesTargets, err := readmeUpdater(options.RepositoryRoot, options.PackageRootPath, options.BuildDir) |
There was a problem hiding this comment.
Could we directly use docs.UpdateReadmes instead of having to pass this function? At the end it makes sense to make the update of readmes part of the build.
| updatedReadmesTargets, err := readmeUpdater(options.RepositoryRoot, options.PackageRootPath, options.BuildDir) | |
| updatedReadmesTargets, err := docs.UpdateReadmes(options.RepositoryRoot, options.PackageRootPath, options.BuildDir) |
There was a problem hiding this comment.
at the installer this step is not done, that is why i passed the function, and at the installer its a noop one to not run the update readme.
we were discussing offline with @mrodm why the installer wasn't updating the readmes. some ideas where to decouple the build function and have one for installing a package and an other for building the package itself.
There was a problem hiding this comment.
at the installer this step is not done, that is why i passed the function, and at the installer its a noop one to not run the update readme.
Oh, I see. Could we make this then a boolean option in BuildOptions instead of having to pass a function?
we were discussing offline with @mrodm why the installer wasn't updating the readmes. some ideas where to decouple the build function and have one for installing a package and an other for building the package itself.
Interesting. Maybe it wouldn't make any harm to update the readmes also on the installer 🤔 I would say that the build process should be the same. But well, no need to change this behavior here.
There was a problem hiding this comment.
updated on the last commit to pass a bool option.
💛 Build succeeded, but was flaky
Failed CI StepsHistory
|
mrodm
left a comment
mrodm
left a comment
There was a problem hiding this comment.
Thanks @teresaromero !
Just tested locally and checked that now the zip files contain the same files as in the source (with the updated markdown files if any).
4 participants
Bug introduced at #2957
When a package is built, the readme is not being updated at the zip bundle. When working on the mentioned PR, the readme updater was moved after the built so the linked fields where already there when doing the update. The bundle zip is already created when this happens, so the readme inside the zip is the one at source before running the updater.
This changes propose to inject the updater function in order to run it after the linked files and the external fields are resolved. This way all the files in the bundle are the updated ones.I've also explored the option of removing from the Builder function the link resolver, but still, the external fields have to be resolved inside the Builder, and if the readme updater runs before this, the readme wont have the external fields documented.The PR adds a new option to update readmes inside the builder function.