Closed drzraf closed 2 years ago
Looks great. Code is much more simpler and more readable
Thanks for the effort!
Two quick notes:
.include()
in Eventizer.js needs to be .includes()
await
s or something moreI'll close #354, let's continue here.
I'll try to remove asyncReadFileFn
(but this impacts preprocess
behavior too) in this very PR, but either way I think it's now mergeable (so we can work on other PR related to generateUniqueIdentifier
, or other enhancements you had in mind @ilessiivi ...)
Do you confirm?
It accomplishes the original goals and it passes all the tests - I would be happy to have this PR merged! Indeed there are still a couple of little things we should do in smaller individual PRs, but I think the project is soon ready for a v3 pre-release (in regards to #353).
@drzraf Do you know why I am getting these emails? What is being deployed with this approve?
Sadly this email does not have a link to a branch or MR which requires this review
The CI runs need to be approved because:
package.json
/ .github/*
or even JS-code using process.env
, ...)That's why the CI process allows for a maintainer to have a look to the diff (regarding CI safety of token/secrets) and, if deemed safe, accept it (to run the the open-env
which contains the secrets)
Reroll and continuation of #354