feat: improve --list-csv and --list output - #1810
Conversation
There was a problem hiding this comment.
1 issue found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/commander.ts">
<violation number="1" location="src/commander.ts:213">
P2: `hasDescriptions` is derived from unfiltered jobs, so `--list` can incorrectly print a `description` column based only on hidden `when: never` jobs.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
b953744 to
36923f2
Compare
|
Having "unstable/non-deterministic" columns in CSV seems like a anti-pattern for people using CSV parsers doesn't it? @bcouetil I can understand that
This id -u fix really should be in a PR of its own. |
|
Hello, nice to talk to you once again 😊
You meant
I wanted to have the column showing independently from activated jobs. In fact, consistency for PEOPLE using the descriptions, and consistency for those not using it. I guessed a on/off usage on a project... I personally do not use it, at all. So my guess is far from perfect. Please tell me your final wish on this.
OK, PR incoming 🫡 |
36923f2 to
f0caf7a
Compare
|
Looks good — description is already dropped from --list-csv in this diff, which addresses my concern. The hasDescriptions/--list point is out of scope for this PR then. |
f0caf7a to
8fbb983
Compare
Sorry, I saw your comment while adding the same to --list... I should not have squashed my commits, but I wanted to share the logic between the too. I kept description on --list, but added both allow_failure and needs change, to have maximum info.
What do you think @firecow ? |
…om csv - Remove description column from --list-csv for stable CSV structure (description is already visible in --list) - Render allowFailure exit_codes as [code1,code2] instead of true/false in both --list and --list-csv - Distinguish null needs (stage ordering, no output) from [] needs (explicitly no dependencies) in both --list and --list-csv - Extract shared private helpers formatAllowFailure and formatNeeds - Update README with comprehensive examples for all cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
8fbb983 to
ac73444
Compare
|
I've now rebased. I think it is finished on my side. |
This MR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Adoption](https://docs.renovatebot.com/merge-confidence/) | [Passing](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---|---|---| | [npm:gitlab-ci-local](https://github.com/firecow/gitlab-ci-local) | `4.70.1` → `4.71.0` |  |  |  |  | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>firecow/gitlab-ci-local (npm:gitlab-ci-local)</summary> ### [`v4.71.0`](https://github.com/firecow/gitlab-ci-local/releases/tag/4.71.0) [Compare Source](firecow/gitlab-ci-local@4.70.1...4.71.0) #### What's Changed - Fix remaining SonarCloud findings by [@​firecow](https://github.com/firecow) in [#​1823](firecow/gitlab-ci-local#1823) - fix(windows): skip id -u on Windows to avoid confusing startup error by [@​bcouetil](https://github.com/bcouetil) in [#​1824](firecow/gitlab-ci-local#1824) - fix: --completion outputs wrong script name when run via Bun by [@​Paul-Goulpie](https://github.com/Paul-Goulpie) in [#​1826](firecow/gitlab-ci-local#1826) - feat: improve --list-csv and --list output by [@​bcouetil](https://github.com/bcouetil) in [#​1810](firecow/gitlab-ci-local#1810) - chore(deps): update all non-major by [@​renovate](https://github.com/renovate)\[bot] in [#​1830](firecow/gitlab-ci-local#1830) - chore(deps): lock file maintenance by [@​renovate](https://github.com/renovate)\[bot] in [#​1827](firecow/gitlab-ci-local#1827) - fix(deps): update dependency re2js to v2 by [@​renovate](https://github.com/renovate)\[bot] in [#​1828](firecow/gitlab-ci-local#1828) #### New Contributors - [@​Paul-Goulpie](https://github.com/Paul-Goulpie) made their first contribution in [#​1826](firecow/gitlab-ci-local#1826) **Full Changelog**: <firecow/gitlab-ci-local@4.70.1...4.71.0> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMzkuMSIsInVwZGF0ZWRJblZlciI6IjQzLjE0MC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWlub3IiXX0=-->


Closes #1809
Changes
[42,137]instead of justtrue/falsein both--listand--list-csv[]when explicitly set to no dependencies — in both--listand--list-csv--list, never in--list-csv(stable CSV structure)formatAllowFailureandformatNeedsto avoid logic duplicationContext
These changes improve the usefulness of
--list-csvfor automated testing of GitLab CI job rules.