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.

Sign up for GitHub

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

Jump to bottom

Add action feed for new release #12324

Merged
merged 17 commits into from
Jul 29, 2020
Merged

Add action feed for new release #12324

merged 17 commits into from
Jul 29, 2020

Conversation

a1012112796
Copy link
Member

@a1012112796 a1012112796 commented Jul 25, 2020 •
edited
Loading

As title

image

@lunny lunny added the type/enhancement An improvement of existing functionality label Jul 25, 2020
@lunny lunny added this to the 1.13.0 milestone Jul 25, 2020
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 25, 2020
@silverwind
Copy link
Member

Maybe omit the downloads? It seems a bit too much information.

@a1012112796
Copy link
Member Author

Maybe omit the downloads? It seems a bit too much information.

I have limited the number of attachments to show here to 5, I think it's enough.

@silverwind
Copy link
Member

Then I'd suggest removing at least the "Downloads" text and unintending the file list to save some space.

@jolheiser
Copy link
Member

jolheiser commented Jul 25, 2020 •
edited
Loading

Why not make attachments a details list, like it is on the actual release page?
Or is it a (closed) drop-down on that page?

@a1012112796
Apply suggestions from code review
451b3af 
* Add ReleaseID to the action table
* Remove error message
* Fold the attachments download list
@lafriks
Copy link
Member

lafriks commented Jul 25, 2020

I also think that no downloads should be shown here. If you need downloads you could click on release and go to release page and download from there.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 26, 2020
@lunny
Copy link
Member

lunny commented Jul 26, 2020

Click 1.0 will jump to the release page?

@a1012112796
Copy link
Member Author

Click 1.0 will jump to the release page?

yes

@a1012112796 a1012112796 requested a review from lafriks July 26, 2020 09:31
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 26, 2020
@6543
Copy link
Member

6543 commented Jul 26, 2020

If you create a release to an existing tag nothing will show up

@a1012112796
Copy link
Member Author

If you create a release to an existing tag nothing will show up

Maybe, I will check it later.

@a1012112796
Copy link
Member Author

a1012112796 commented Jul 26, 2020 •
edited
Loading

@6543 resolved

@6543
Copy link
Member

6543 commented Jul 26, 2020

Wow that was fast!

@lafriks
Copy link
Member

lafriks commented Jul 26, 2020 •
edited
Loading

Release title would be usually same as tag imho so that should be shown in the link text instead of tag name. Just like it is shown in release list

6543
6543 requested changes Jul 26, 2020
View reviewed changes
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

ok ☝️ for function.

But @lafriks is right - we should use the real title instead of the tag
and I would remove the detail (where the title is put now) since you can have a look at the release when you klik at the link

lafriks
lafriks reviewed Jul 27, 2020
View reviewed changes
Copy link
Member

@lafriks lafriks left a comment

Choose a reason for hiding this comment

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

I meant something like this (did not test tho :) )

templates/user/dashboard/feeds.tmpl Outdated Show resolved Hide resolved
options/locale/locale_en-US.ini Outdated Show resolved Hide resolved
templates/user/dashboard/feeds.tmpl Outdated Show resolved Hide resolved
@a1012112796
Copy link
Member Author

Maybe this pr should have bug label because of 6622464

@6543

This comment has been minimized.

Sign in to view
6543
6543 approved these changes Jul 27, 2020
View reviewed changes
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

never mind

🚀

@6543
Copy link
Member

6543 commented Jul 28, 2020

🚀

@techknowlogick
Copy link
Member

🚀

@techknowlogick techknowlogick merged commit 1f12dc8 into go-gitea:master Jul 29, 2020
@6543
Copy link
Member

6543 commented Jul 29, 2020

@a1012112796 can you backport the bugfix part?

@a1012112796 a1012112796 mentioned this pull request Jul 30, 2020
Closed
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lafriks lafriks lafriks approved these changes

@6543 6543 6543 approved these changes

@silverwind silverwind silverwind approved these changes

@lunny lunny lunny approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.13.0
Development

Successfully merging this pull request may close these issues.
8 participants
@a1012112796 @silverwind @jolheiser @lafriks @lunny @6543 @techknowlogick @GiteaBot