Justfile: make running the graph-builder and e2e tests simpler #258
Conversation
openshift-ci-robot
added
the
approved
|
/retest |
LalatenduMohanty
left a comment
LalatenduMohanty
left a comment
There was a problem hiding this comment.
IMO this should be a single commit.
Justfile
Outdated
There was a problem hiding this comment.
We need to add the graph.svg to .gitignore.
There was a problem hiding this comment.
For some reason this is not working in my tmux session. But thats not a blocker for the PR.
There was a problem hiding this comment.
Please paste the command and its output here. It's supposed to work for you as well.
There was a problem hiding this comment.
I guess I am using $ just display-graph in a wrong way. Can you add an example in the dev docs?
There was a problem hiding this comment.
Running $ just get-and-display-graph works fine. But as expected $ just display-graph would hang as it need json input. If this is right, we should make $ just display-graph a private recipe.
There was a problem hiding this comment.
@steveej mentioned that the right usage is just get-graph-pe-production "stable-4.2" "amd64" | just display-graph. As discussed we should add docs for display-graph in the Justfile for just --list to show. I will also suggest that we add just --list to the dev docs.
There was a problem hiding this comment.
I'm not yet ready to document all of this. I would like to, but I'm not convinced this is a stable interface. Right now we're observing a divergence, or at best a duplication, between the Justfile and the scripts in dist/ and hack/. I would like to unify this as much as possible soon along with updating the documentation.
There was a problem hiding this comment.
I think the divergence exists because we have not accepted the Justfile yet as the default way for local development. By making it easier to use the Justfile will be a positive step in that direction. I would suggest lets add the doc required for this PR in $ just --list and then take up the other things required to unify things.
There was a problem hiding this comment.
Okay. So you're suggesting to simply add docs to the recipes in this PR?
I don't think so as they're unrelated changes. But if you feel strongly about it I'd rather squash than having this PR linger around 😉 |
steveej
force-pushed
the
pr/justfile-gb-improvements
branch
from
a7dc270 to
417ba3c
Compare
I do not have a strong opinion. |
Justfile
Outdated
There was a problem hiding this comment.
I guess I am using $ just display-graph in a wrong way. Can you add an example in the dev docs?
steveej
force-pushed
the
pr/justfile-gb-improvements
branch
from
417ba3c to
ba6e454
Compare
This also mentions it in the developer docs
steveej
force-pushed
the
pr/justfile-gb-improvements
branch
from
ba6e454 to
cf34b32
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: LalatenduMohanty, steveeJ The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
No description provided.