URL encoding in query function and refactor #105
Conversation
JacksonChen666
force-pushed
the
url-encode-the-world
branch
from
2034e5d to
2fc4ae1
Compare
There was a problem hiding this comment.
Thanks a lot for this! LGTM except some additional documentation ideas within.
And one more thing about contributing docs but that let be my concern, you shouldn't be bothered with it! :-)
We need to update the Implementation Examples chapter, it is based on very old commits and just not the reality anymore. Now that f-strings are not allowed to use anymore it is even more wrong. I will take care of fixing that!
So as mentioned within the code, your part would be to add a tiny new chapter about the new string formatting concept. Thanks a lot!
| @@ -297,7 +311,8 @@ def room_get_id(self, room_alias): | |||
| response as it might contain Synapse's error message. | |||
| """ | |||
| room_directory = self.query( | |||
| "get", f"client/r0/directory/room/{urllib.parse.quote(room_alias)}" | |||
| "get", "client/r0/directory/room/{room_alias}", | |||
| room_alias=room_alias | |||
There was a problem hiding this comment.
I think it would be good if we state this new concept somewhere in the contributing docs. Telling the contributors about the new handling of args. I'm not quite sure where this should be.
Around here a new chapter around string formatting maybe? https://github.com/JOJ0/synadm/blob/master/CONTRIBUTING.md#command-design
There was a problem hiding this comment.
@JacksonChen666 if you want and think it's ready and tested enough we can merge this. I can take over the docs part. I would fix the links in this chapter https://github.com/JOJ0/synadm/blob/master/CONTRIBUTING.md#implementation-examples to point to a current version of these cli/user.py things and explain the passing as args and kwargs in a new PR that we can refine together then.
There was a problem hiding this comment.
not ready to merge (conflicts and outdated dev branch mess), but i'll hand off documentation to you then.
JacksonChen666
force-pushed
the
url-encode-the-world
branch
from
2fc4ae1 to
fa8c353
Compare
|
oops included my extra stuff, rebasing... |
1. query now accepts other things as part of str.format 2. refactor all use of the query function in the project 3. function documentation could be much better 4. needs documentation on how to use query function
JacksonChen666
force-pushed
the
url-encode-the-world
branch
from
fa8c353 to
4f12233
Compare
|
should be good to merge |
resolves #96
Things to look out for in this PR:
str.formatdoesn't break because it wasn't given enough things