-
Notifications
You must be signed in to change notification settings - Fork 27.4k
Conversation
Removes a "magic number" (2) used multiple times in the code. Removes unnecessary variables "arg" and "prefix". Removed a condition within the "for" loop that generates query string parameters. Adds a test case to ensure URL parameteres are generated as expected.
@LeonardoBraga can you show an example of the improvement? It seems like it's more just cleanup.. |
@ilanbiala You're correct, one can say it's more of a cleanup that removes a few things and changes others. The changes I mentioned in the comment are the (minor) improvements I referred to, plus the test case for the error URL parameters. This is my first PR, so please accept my apologies if the term "improvement" wasn't a good fit for the description of these changes. |
@LeonardoBraga it's fine, you might just want to edit the PR title for posterity. |
@ilanbiala Done. Please let me know if you have another suggestion for it. Thanks. |
After applying the changes requested by @ilanbiala, I would like to know if there are any other adjustments or questions that might be holding this PR. Thanks! |
@LeonardoBraga will review this in the next day or two |
Thanks, @lgalfaso! |
@lgalfaso did you have the opportunity to take a look at this one? |
landed as b146cae |
Removes a "magic number" (2) used multiple times in the code.
Removes unnecessary variables "arg" and "prefix".
Removed a condition within the "for" loop that generates query string parameters.
Adds a test case to ensure URL parameters are generated as expected.