Consolidating a few past PRs #302
Conversation
- Fixed broken URLs from PR 240 - Added example from PR 231 - Added note about Key Manager from PR 285 - Removed references to, and changed diagrams concerning, Node Resolver Signed-off-by: Quintessence <[email protected]>
quintessence
requested review from
sanderson042,
mchurichi,
ajessup,
Andres-GC,
evan2645 and
umairmkhan
as code owners
✅ Deploy Preview for spiffe ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
For the server diagram, a couple things:
|
|
Hey @mchurichi would you mind reviewing these? :) |
There was a problem hiding this comment.
Thank you @quintessence for combining all these changes! I've just left a few comments.
- Fixed branding URLs - Re-added VMWare Secrets Manager Signed-off-by: Quintessence <[email protected]>
quintessence
force-pushed
the
urls-node-resolver
branch
from
6ee966d
to
37c2460
Compare
Signed-off-by: Quintessence <[email protected]>
|
Hello! I saw that main was pulled into the branch and wanted to break out (for ease of reading) what needs to be resolved on the PR so it can be merged :) |
quintessence
force-pushed
the
urls-node-resolver
branch
from
1816c99
to
2e39585
Compare
|
I sync’d up with @quintessence last week on this, made a suggestion which I think has been accepted. Also, just to remove the conversation around DCO and attribution, we backed out the commit related to #285 ... also had a look at #262 which included some changes now found in this PR, but those changes were limited to focused deletions etc .. I thought it’s probably fine to include, but let me know if you prefer to cherry pick there as well |
There was a problem hiding this comment.
There's a minor issue with a few missing periods that could optionally be fixed.
I also wanted to flag a possible security concern. It's probably nothing, but maybe someone with more expertise should review it. @evan2645 ?
quintessence
force-pushed
the
urls-node-resolver
branch
2 times, most recently
from
41e6f6b
to
7879fb7
Compare
|
It looks like commit 7879fb7 has a failed DCO, even though the signature is there and is also on prior commits. Is there a way to re-trigger the DCO check? A bot command to run or similar? |
|
I'm wondering if it's reading the "fail" from 37c2460 as a DCO failure - but it wasn't, the Deploy Preview failed there for some reason. |
quintessence
force-pushed
the
urls-node-resolver
branch
from
7879fb7
to
12e8a8f
Compare
|
Tried another force push with signature just in case, but alas no 🤔 |
Signed-off-by: Quintessence <[email protected]>
Signed-off-by: Quintessence <[email protected]>
Co-authored-by: Steve Anderson <[email protected]> Signed-off-by: Quintessence <[email protected]>
Co-authored-by: Steve Anderson <[email protected]> Signed-off-by: Quintessence <[email protected]>
Signed-off-by: Quintessence <[email protected]>
Signed-off-by: Quintessence <[email protected]>
quintessence
force-pushed
the
urls-node-resolver
branch
from
12e8a8f
to
217856f
Compare
|
@quintessence thanks for fixing the commit history :) this looks good to me, it just needs to be updated with the latest changes from master and that should be it. |
There was a problem hiding this comment.
Hi @quintessence - Approved! 🚀
Thank you very much for all your work on this. You really persevered against a variety of technical and logistical issues. Thanks Maxi and Evan for your help, too!
|
🎉 |
Description of the change
Fixing broken URLs that were fixed in other open PRs (listed below) and also removed references to Node Resolver (PR #262 ).
Which issue this PR fixes
PR adding further description of KeyManager #285