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

Refactor Issues Subscription #8738

Merged
merged 37 commits into from
Nov 20, 2019

Conversation

6543
Copy link
Member

@6543 6543 commented Oct 29, 2019

@6543 6543 changed the title Refactor Issues Subscription [WIP] Refactor Issues Subscription Oct 29, 2019
@6543 6543 force-pushed the refactor_issues-subscription branch from f7a788d to aa31558 Compare November 1, 2019 05:21
models/issue_watch.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 6, 2019
@6543
Copy link
Member Author

6543 commented Nov 6, 2019

move ToDos into new issue: #8852

to make refactor independend from feature additions

models/fixtures/issue_watch.yml Outdated Show resolved Hide resolved
@6543 6543 changed the title [WIP] Refactor Issues Subscription Refactor Issues Subscription Nov 15, 2019
@6543 6543 force-pushed the refactor_issues-subscription branch from 37d0624 to 0ac149a Compare November 15, 2019 22:30
@guillep2k
Copy link
Member

It's no longer leaking the mail addresses through this endpoint. However, there's no longer a way for admins to get the users' addresses through most endpoints including this (/admin/adminGetAllUsers seems to work). I don't know, looks like a breaking change?

@6543
Copy link
Member Author

6543 commented Nov 17, 2019

@guillep2k the PR dont change API behaviour only fix a bug and refactor code ...

@6543
Copy link
Member Author

6543 commented Nov 17, 2019

@lunny this PR is ready ...

@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 Nov 17, 2019
@lunny
Copy link
Member

lunny commented Nov 17, 2019

Make L-G-T-M work

routers/api/v1/api.go Show resolved Hide resolved
@guillep2k
Copy link
Member

force-pushed the 6543-forks:refactor_issues-subscription branch 2 times, most recently from 37d0624 to 0ac149a 2 days ago

@6543 first force-push fixed the user email leaking, second force-push removed the fix? I can't find that fix anymore.

@6543
Copy link
Member Author

6543 commented Nov 17, 2019

@guillep2k I removed this "fix" - because there was no bug user.apiformat already handle this for us ...

@6543 6543 requested a review from lunny November 18, 2019 07:10
@6543
Copy link
Member Author

6543 commented Nov 18, 2019

@lunny can you look at #8738 (comment) plz
@guillep2k #8738 (comment) plz

@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Nov 19, 2019
@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 Nov 20, 2019
@lunny lunny merged commit 2ab8c78 into go-gitea:master Nov 20, 2019
@6543 6543 deleted the refactor_issues-subscription branch November 20, 2019 15:10
@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.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants