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

Disable Stars config option #14653

Merged
merged 14 commits into from
Apr 15, 2021
Merged

Conversation

kdumontnu
Copy link
Contributor

If DISABLE_STARS = true remove the option to star repositories

image

The "Starred Repositories" tab in user profile is replaced with "Watched Repositories"
image

This is helpful for enterprise/private accounts where the ability to "star" repositories is not very useful and creates confusion compared to "watching".

Note: I'm working on a separate PR to toggle Watch status in the Profile repo list

@6543
Copy link
Member

6543 commented Feb 11, 2021

I think the "watching repos" is a nice func anyway :) ... just if we going to add it, can we add a prifacy opt-out add too?

like it exist for Hide the activity from the profile page

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 11, 2021
@6543 6543 added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Feb 11, 2021
@6543 6543 added this to the 1.15.0 milestone Feb 11, 2021
@kdumontnu
Copy link
Contributor Author

I think the "watching repos" is a nice func anyway :) ... just if we going to add it, can we add a prifacy opt-out add too?

like it exist for Hide the activity from the profile page

I used the same settings as "Starred Repositories". Should that be hidden as well? The setting is also ignored for Repositories, Following, and Followed. This seems like a separate PR, but I'm happy to open one to fix that.

@techknowlogick techknowlogick changed the title Kd/conf disable stars Disable Stars config option Feb 11, 2021
@codecov-io
Copy link

codecov-io commented Feb 14, 2021

Codecov Report

Merging #14653 (2638218) into master (487f2ee) will decrease coverage by 0.00%.
The diff coverage is 34.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14653      +/-   ##
==========================================
- Coverage   42.21%   42.21%   -0.01%     
==========================================
  Files         767      767              
  Lines       81624    81652      +28     
==========================================
+ Hits        34458    34468      +10     
- Misses      41531    41547      +16     
- Partials     5635     5637       +2     
Impacted Files Coverage Δ
models/repo_list.go 77.19% <0.00%> (-1.57%) ⬇️
modules/setting/repository.go 57.69% <ø> (ø)
modules/structs/issue.go 0.00% <ø> (ø)
routers/user/profile.go 43.24% <0.00%> (-4.05%) ⬇️
modules/queue/unique_queue_disk_channel.go 55.22% <50.00%> (+1.37%) ⬆️
modules/context/context.go 58.61% <100.00%> (+0.10%) ⬆️
modules/setting/setting.go 49.03% <100.00%> (ø)
modules/templates/base.go 42.30% <100.00%> (+1.13%) ⬆️
routers/api/v1/settings/settings.go 100.00% <100.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f05a28...8bf3a61. Read the comment docs.

@kdumontnu
Copy link
Contributor Author

@6543 did you still want me to make the change to put this behind the KeepActivityPrivate flag?
It appears right now this flag is only checked for the Activity tab, not Repositories, Stars, Following, or Followed.
I'm happy to make the change, but honestly I think it's better to do in a separate PR to address all cases.

@6543
Copy link
Member

6543 commented Feb 16, 2021

@kdumontnu I think we can seperate it ...

@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 Feb 20, 2021
@6543
Copy link
Member

6543 commented Feb 21, 2021

@kdumontnu sorry that it wont get reviewed and merged right away... just for you as an info: gitea is in feature freeze at the moment ...

@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 Feb 25, 2021
@zeripath
Copy link
Contributor

OK I think this looks fine - I don't quite understand why the feature needs disabling but the PR does do everything needed to disable the feature completely.

@kdumontnu
Copy link
Contributor Author

OK I think this looks fine - I don't quite understand why the feature needs disabling but the PR does do everything needed to disable the feature completely.

Excellent! I understand the hesitation, but generally speaking for private projects "stars" provide very little value and can add unnecessary complexity for users.

@techknowlogick
Copy link
Member

@kdumontnu just reaching out to you here to let you know I sent a PM to your discord, hoping you saw it :)

@zeripath
Copy link
Contributor

Please resolve conflicts

@6543
Copy link
Member

6543 commented Apr 15, 2021

@kdumontnu can you "update branch" please?

@6543
Copy link
Member

6543 commented Apr 15, 2021

🚀

@6543 6543 merged commit f44543a into go-gitea:master Apr 15, 2021
@kdumontnu kdumontnu deleted the kd/conf_disable_stars branch May 3, 2021 23:24
@go-gitea go-gitea locked and limited conversation to collaborators Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants