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

Move admin endpoints into separate files #6308

Merged

Conversation

awesome-manuel
Copy link
Contributor

@awesome-manuel awesome-manuel commented Oct 31, 2019

@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
Copy link
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
Member

@awesome-manuel are you still around?

@awesome-manuel
Copy link
Contributor Author

@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
Contributor Author

@anoadragon453 can you trigger buildkite please?

@richvdh
Copy link
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
Member

richvdh commented Nov 20, 2019

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

@awesome-manuel
Copy link
Contributor Author

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
Signed-off-by: Manuel Stahl <manuel.stahl@awesome-technologies.de>
Signed-off-by: Manuel Stahl <manuel.stahl@awesome-technologies.de>
Signed-off-by: Manuel Stahl <manuel.stahl@awesome-technologies.de>
Signed-off-by: Manuel Stahl <manuel.stahl@awesome-technologies.de>
@awesome-manuel
Copy link
Contributor Author

@richvdh @anoadragon453 everything is fixed now.

Copy link
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
* 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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants