Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix popup when a CL doesn't have revisions #53

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

avm99963
Copy link

When a CL doesn't have revisions, the current revision can't be found and thus we can't retrieve the current revision's description.

This CL fixes this by returning null in getDescription() when this happens, and falling back to the change's subject in the CL widget.

Fixes #52

When a CL doesn't have revisions, the current revision can't be found
and thus we can't retrieve the current revision's description.

This CL fixes this by returning null in getDescription() when this
happens, and falling back to the change's subject in the CL widget.

Fixes sdefresne#52
@avm99963
Copy link
Author

avm99963 commented Mar 5, 2024

@sdefresne Friendly ping in case you didn't see it (in another PR you mention you didn't receive an email notification for the PR, maybe tagging you sends it :)

Thanks in advance! :))

Copy link
Owner

@sdefresne sdefresne left a comment

Choose a reason for hiding this comment

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

Sorry for the long delay. The notifications goes to my personal email, and they got lost amongst the other emails.

The change looks good, though I think it could be simpler by creating a Description(...) object with an empty message.

if (currentRevision !== undefined) {
this.description_ = new Description(currentRevision.commit.message);
} else {
this.description_ = null;
Copy link
Owner

Choose a reason for hiding this comment

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

Could we instead return new Description("")?

IIUC the code, this should be enough and won't require the other changes.

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.

Error when a change doesn't have any revision
2 participants