Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Move admin endpoints into separate files#6308

Merged
anoadragon453 merged 4 commits intomatrix-org:developfrom
Awesome-Technologies:move_admin_endpoints
Nov 20, 2019
Merged

Move admin endpoints into separate files#6308
anoadragon453 merged 4 commits intomatrix-org:developfrom
Awesome-Technologies:move_admin_endpoints

Conversation

@awesome-manuel
Copy link
Copy Markdown
Contributor

@awesome-manuel awesome-manuel commented Oct 31, 2019 •
edited
Loading

@richvdh @erikjohnston this is a prerequisite for #5925

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file
  • Pull request includes a sign off

@awesome-manuel awesome-manuel changed the base branch from master to develop October 31, 2019 12:09
@anoadragon453 anoadragon453 self-assigned this Nov 5, 2019
anoadragon453
anoadragon453 reviewed Nov 5, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Looks good. You'll need to do a couple changes to fix the CI though.

First, you'll need to pull in the latest changes from develop to fix a pep 517 issue with python on the sample config script.

Next, you'll want to run ./scripts-dev/lint.sh to correct isort and any other linting issues. Make sure you've got your linting tools up to date as well. For instance, the version of black that we use is 19.10b0 (I don't think the others have changed recently).

Finally, you'll need to include a changelog file. I recommend a .misc extension for it.

@anoadragon453
Copy link
Copy Markdown
Member

anoadragon453 commented Nov 18, 2019

@awesome-manuel are you still around?

@awesome-manuel
Copy link
Copy Markdown
Contributor Author

awesome-manuel commented Nov 19, 2019

@awesome-manuel are you still around?

Yes. The PR was rebased to develop when I updated it. There also were no linting issues at that time.
But I can update it again to the latest develop, hopefully this week.

@awesome-manuel
Copy link
Copy Markdown
Contributor Author

awesome-manuel commented Nov 19, 2019

@anoadragon453 can you trigger buildkite please?

@richvdh
Copy link
Copy Markdown
Member

richvdh commented Nov 20, 2019

@awesome-manuel I triggered it; it should run automatically in future. Looks like the CI is very unhappy about something - could you investigate?

@richvdh
Copy link
Copy Markdown
Member

richvdh commented Nov 20, 2019

mostly builtins.ImportError: cannot import name 'assert_user_is_admin' apparently.

@awesome-manuel
Copy link
Copy Markdown
Contributor Author

awesome-manuel commented Nov 20, 2019

mostly builtins.ImportError: cannot import name 'assert_user_is_admin' apparently.

Strange, since assert_requester_is_admin is imported the same way and has no problems...

@awesome-manuel awesome-manuel force-pushed the move_admin_endpoints branch 2 times, most recently from 0b0e53d to 9b03f7c Compare November 20, 2019 09:34
@awesome-manuel
Copy link
Copy Markdown
Contributor Author

awesome-manuel commented Nov 20, 2019

@richvdh @anoadragon453 everything is fixed now.

anoadragon453
anoadragon453 approved these changes Nov 20, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you for you contribution!

@anoadragon453 anoadragon453 merged commit 4f5ca45 into matrix-org:develop Nov 20, 2019
@awesome-manuel awesome-manuel deleted the move_admin_endpoints branch December 18, 2019 12:56
babolivier pushed a commit that referenced this pull request Sep 1, 2021
@anoadragon453
Move admin endpoints into separate files (#6308)
a148286 
* commit '4f5ca455b':
  Move admin endpoints into separate files (#6308)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@anoadragon453 anoadragon453 anoadragon453 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@anoadragon453 anoadragon453

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@awesome-manuel @anoadragon453 @richvdh