Fix outdated docs and add "URL encoding chapter" #109
Conversation
|
As usual, it's longer than I thought it will be. Give me your critics! ;-) Best viewed in its rendered form in the dev branch: https://github.com/JOJ0/synadm/blob/dev/CONTRIBUTING.md#command-design I tried to follow the wording from the Python docs: https://docs.python.org/3.9/library/string.html#formatstrings |
|
|
||
| Since version 0.42 `synadm` encodes URL's in a central place - the `ApiRequest.query()` function in the `synadm.api` module. | ||
|
|
||
| Variables sent as part of the URL are required to be passed to the `query()` method **unaltered**. Do not use f-strings or str.format, let the `query()` method do the sanitizing of the URL. |
There was a problem hiding this comment.
Add negative (unwanted) example and reason to do so (#96)?
(It would likely have to be part of the CONTRIBUTING.md itself instead of code in the repo)
There was a problem hiding this comment.
A negative example additionally to the positive one would certainly reassure that it is understood. I'll think about it.
CONTRIBUTING.md
Outdated
|
|
||
| Variables sent as part of the URL are required to be passed to the `query()` method **unaltered**. Do not use f-strings or str.format, let the `query()` method do the sanitizing of the URL. | ||
|
|
||
| If we take a look at the `user password` example in above's chapter, we have: |
There was a problem hiding this comment.
| If we take a look at the `user password` example in above's chapter, we have: | |
| If we take a look at the `user_password` example in above's chapter, we have: |
There was a problem hiding this comment.
Hmm I wanted to "refer to" the synadm user password command example. So maybe I was misleading with that.
CONTRIBUTING.md
Outdated
|
|
||
| ### Submitting data & URL encoding | ||
|
|
||
| Since version 0.42 `synadm` encodes URL's in a central place - the `ApiRequest.query()` function in the `synadm.api` module. |
There was a problem hiding this comment.
the
ApiRequest.query()function
but that's not how we call the query function, right?
CONTRIBUTING.md
Outdated
|
|
||
| ### Submitting data & URL encoding | ||
|
|
||
| Since version 0.42 `synadm` encodes URL's in a central place - the `ApiRequest.query()` function in the `synadm.api` module. |
There was a problem hiding this comment.
The thing has never been included in a release, yet. Referring to it specifically by commit SHA
| Since version 0.42 `synadm` encodes URL's in a central place - the `ApiRequest.query()` function in the `synadm.api` module. | |
| Since commit [6874939](https://github.com/JOJ0/synadm/commit/68749391d6a291d2fac229214f59924189c775ac), `synadm` encodes URL's in a central place - the `ApiRequest.query()` function in the `synadm.api` module. |
There was a problem hiding this comment.
We will release this in 0.42 and then it stays correct forever.
There was a problem hiding this comment.
checks commit logs since v0.41.1 to dev branch
J0J0 Todos (7):
Merge pull request #108 from JOJ0/default-draft-releases
Update "Implement. Examples" chapter code-links
Update "Helpers" chapter on CONTRIBUTING page
Add URL-encoding chapter to CONTRIBUTING page
Fix typos in URL-encoding chapter
Apply suggestions from code review
Apply suggestions from code review
Jackson Chen (2):
make auto releases draft by default
URL encoding in query function and refactor
Nope, doesn't seem like a minor release to me.
I think the commit should probably be included anyways alongside the commit itself, seems like a reasonable compromise of "when".
There was a problem hiding this comment.
I would've called it a minor release since it kind of "revolutionises" quite some internals and might even introduce bugs we couldn't think of yet (Well, that is esoterically, I don't really think it will introduce bugs).
But I see your point that it actually doesn't bring any new feature, it just fixes a bug and prevents future issues.
There was a problem hiding this comment.
Yep you are right, it really doesn't hurt to link to the commit. If someone wants details, they can find them easily. , commiting right away....
Thanks a lot! Nitpicking appreciated :-) |
on CONTRIBUTING page. Permalink to most current commits and display full function compared to just a snippet, as we had previously.
Link to simply api.py without a line number. Line numbers will change. In this case not useing permalinks to commits. The goal is showing the most recent version of the available helpers and utils. Description points to where to find them already.
on CONTRIBUTING page.
Co-authored-by: Jackson Chen <[email protected]>
Co-authored-by: Jackson Chen <[email protected]>
to CONTRIBUTION docs page.
to state the exact point in time the query method's url-encoding feature was introduced.
in CONTRIBUTING.md.
|
Unfortunately I forgot to finish this one. There might have been some things that still could be improved but due to lack of time I'm deciding now to better merge it than have inaccurate docs any time longer. I read through it and think it should be understandable by our contributors. |
This fixes links to code snippets in CONTRIBUTING.md chapter "Implementation Examples" and adds a chapter about the newly introduced way of formatting strings in the
query()method ofApiRequest.