Skip to content

Diff: Use doc_name when returned by the CLI#607

Open
Polo2 wants to merge 1 commit into
mainfrom
diff--read-doc_name
Open

Diff: Use doc_name when returned by the CLI#607
Polo2 wants to merge 1 commit into
mainfrom
diff--read-doc_name

Conversation

@Polo2

@Polo2 Polo2 commented Jun 11, 2026

Copy link
Copy Markdown
Member

cf related PR on CLI bump-sh/cli#828

When diff is requested to the CLI,
doc.id may has been used as doc parameter.

In this case, GitHub comment is not easy to read,
api.id has no sense on this comment.

We add a test in the docName logic (moved in its own function as tiny refacto), and favor DiffResponse.doc_name when provided.

About retro-compatibiliy:
Even if DiffResponse.doc_name will be included in the next release of bump-cli, field is missing for previous versions.

I handled the retro-compatibility with a
bump.DiffResponse & { doc_name?: string }
but I'm not very proud, WDYT dear reviewer?

cf related PR on CLI bump-sh/cli#828

When diff is requested to the CLI,
doc.id may has been used as doc parameter.

In this case, GitHub comment is not easy to read,
api.id has no sense on this comment.

We add a test in the docName logic (moved in its own function as tiny refacto),
and favor DiffResponse.doc_name when provided.

About retro-compatibiliy:
Even if DiffResponse.doc_name will be included in the next release
of bump-cli, field is missing for previous versions.

I handled the retro-compatibility with a
`bump.DiffResponse & { doc_name?: string }`
but I'm not very proud, WDYT dear reviewer?
Comment thread src/diff.ts
}

function docName(
diff: DiffResponse & { doc_name?: string },

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cf PR description

About retro-compatibiliy:
Even if DiffResponse.doc_name will be included in the next release of bump-cli, field is missing for previous versions.

I handled the retro-compatibility with a bump.DiffResponse & { doc_name?: string } but I'm not very proud, WDYT dear reviewer?

@Polo2 Polo2 self-assigned this Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant