Skip to content

Suggest creating a new release after merging a PR in first-published-tag-for-merged-pr#5769

Merged
fregante merged 1 commit intomainfrom
new-tag-after-pr
Jul 3, 2022
Merged

Suggest creating a new release after merging a PR in first-published-tag-for-merged-pr#5769
fregante merged 1 commit intomainfrom
new-tag-after-pr

Conversation

@fregante
Copy link
Copy Markdown
Member

@fregante fregante commented Jul 3, 2022 •
edited
Loading

Test URLs

Screenshot

Screen.Recording.2.mov

fregante
fregante commented Jul 3, 2022
View reviewed changes
Comment thread source/features/first-published-tag-for-merged-pr.tsx
if (tagName) {
addExistingTagLink(tagName);
} else if (canCreateRelease()) {
addLinkToCreateRelease('This PR doesn’t appear to have been released yet');
Copy link
Copy Markdown
Member Author

@fregante fregante Jul 3, 2022

Choose a reason for hiding this comment

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

Screen Shot 12

Comment thread source/github-events/on-pr-merge.ts

import observeElement from '../helpers/simplified-element-observer';

export default function onPrMerge(callback: VoidFunction): Deinit {
Copy link
Copy Markdown
Member Author

@fregante fregante Jul 3, 2022

Choose a reason for hiding this comment

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

Can be used in pr-branch-auto-delete too

Comment thread source/github-helpers/timeline-item.tsx
@@ -0,0 +1,6 @@
import React from 'dom-chef';

export default function TimelineItem(): JSX.Element {
Copy link
Copy Markdown
Member Author

@fregante fregante Jul 3, 2022

Choose a reason for hiding this comment

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

Overlap with #5766

@fregante fregante mentioned this pull request Jul 3, 2022
Merged
@fregante fregante merged commit fb52f34 into main Jul 3, 2022
@fregante fregante deleted the new-tag-after-pr branch July 3, 2022 16:27
@fregante
Copy link
Copy Markdown
Member Author

fregante commented Jul 3, 2022

Merged but feel free to review and suggest copy changes, etc. I just wanted to unblock #5766

@fregante fregante changed the title Suggest creating a new release after merging a PR Suggest creating a new release after merging a PR in "first-published-tag-for-merged-pr" Jul 4, 2022
@fregante fregante changed the title Suggest creating a new release after merging a PR in "first-published-tag-for-merged-pr" Suggest creating a new release after merging a PR in first-published-tag-for-merged-pr Jul 4, 2022
@fregante fregante mentioned this pull request Jul 4, 2022
Merged
@akien-mga
Copy link
Copy Markdown

akien-mga commented Jul 8, 2022 •
edited
Loading

Would it be possible to have separate options to configure this new behavior and showing the first published tag with a merged PR?

I found the latter very convenient, but the new suggestion to create a release is really going in the way for a repo where I merge dozens of PRs daily and make tagged releases very infrequently (https://github.com/godotengine/godot).

@fregante
Copy link
Copy Markdown
Member Author

fregante commented Jul 8, 2022 •
edited
Loading

really going in the way

It's a notice, you can ignore it. I don't see it as a big issue.

As a bonus, you can also hide it by pasting this in the custom CSS section

.ml-0.pl-0.ml-md-6.pl-md-3.mt-3 .flash:not(.flash-success) {
	display: none;
}

@yakov116
Copy link
Copy Markdown
Member

yakov116 commented Jul 8, 2022

Come to think of it why is it part of one feature?

@fregante
Copy link
Copy Markdown
Member Author

fregante commented Jul 8, 2022

Because they do the same thing:

  • has this PR been released?
    • yes: link to it
    • no: can the current user release it?
      • yes: link to new release
      • no: 🤫

@fregante fregante mentioned this pull request Jul 11, 2022
Closed
@lode
Copy link
Copy Markdown

lode commented Jul 23, 2022

Not sure if this is the place to discuss, but I also find the suggest to create a release quite annoying. Mostly on a private repo which doesn't have any releases and won't get any in the future either. Is it possible to hide it when the repo doesn't use releases at all?

I also find it a little hard to hide via css, since the code example posted seems very fragile.

@yakov116
Copy link
Copy Markdown
Member

yakov116 commented Jul 24, 2022

Can you make a new issue?

@fregante
Copy link
Copy Markdown
Member Author

fregante commented Jul 24, 2022

I don’t get the point. There are a hundred buttons that you don’t use on the page, are they all tempting you? There’s a big “Close and comment” button in every conversation, but you rarely do that. Should it be hidden? 🤷‍♂️

@yakov116
Copy link
Copy Markdown
Member

yakov116 commented Jul 24, 2022

This is a banner ot sticks out

@fregante fregante mentioned this pull request Jul 25, 2022
Merged
@lode lode mentioned this pull request Jul 25, 2022
Closed
@fregante fregante mentioned this pull request Sep 15, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Assist with releasing from pull request

4 participants

@fregante @akien-mga @yakov116 @lode