Skip to content

Conversation

yakov116
Copy link
Member

@yakov116 yakov116 commented Jul 7, 2022

Really this should be renamed to isClosedConversation but...

@fregante
Copy link
Member

fregante commented Jul 7, 2022

Really this should be renamed to isClosedConversation but...

You're right, we're always using it as an exclusion on PRs that already have PR-only include anyway.

Can you deprecate isClosedPR, leave it unchanged and add isClosedConversation?

We can drop isClosedPR on the next major release (not now)

@yakov116
Copy link
Member Author

yakov116 commented Jul 7, 2022

Random.... Online editor is Awesome!! Love the . it works so well.

@yakov116
Copy link
Member Author

yakov116 commented Jul 7, 2022

🚀 ?

@yakov116 yakov116 changed the title Add "Status: Closed as not planned" to isClosedPR Add "Status: Closed as not planned" to isClosedConversation Jul 7, 2022
index.ts Outdated
export const isClosedConversation = (): boolean => exists('#partial-discussion-header :is([title="Status: Closed"], [title="Status: Merged"], [title="Status: Closed as not planned]")');
/**
@deprecated Use isClosedConversation
*/
Copy link
Member

Choose a reason for hiding this comment

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

/** */ also works on one line

@fregante fregante changed the title Add "Status: Closed as not planned" to isClosedConversation Add isClosedConversation Jul 7, 2022
Copy link
Member Author

@yakov116 yakov116 left a comment

Choose a reason for hiding this comment

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

One line

@yakov116 yakov116 merged commit e339286 into main Jul 7, 2022
@yakov116 yakov116 deleted the ClosedAsNotPlanned branch July 7, 2022 16:09
@yakov116
Copy link
Member Author

yakov116 commented Jul 7, 2022

Um this PR makes GitHub crash 🎉.

Maybe the :is() should not be used here?

@yakov116
Copy link
Member Author

yakov116 commented Jul 7, 2022

Maybe the :is() should not be used here?

Yup
@fregante why?

@yakov116
Copy link
Member Author

yakov116 commented Jul 7, 2022

Ouch I see why...

export const isOpenPR = (): boolean => exists('#partial-discussion-header [title="Status: Open"], #partial-discussion-header [title="Status: Draft"]');
export const isMergedPR = (): boolean => exists('#partial-discussion-header [title="Status: Merged"]');
export const isClosedPR = (): boolean => exists('#partial-discussion-header [title="Status: Closed"], #partial-discussion-header [title="Status: Merged"]');
export const isClosedConversation = (): boolean => exists('#partial-discussion-header :is([title="Status: Closed"], [title="Status: Merged"], [title="Status: Closed as not planned]")');
Copy link
Member

Choose a reason for hiding this comment

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

Oops

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants