falconry / falcon

The no-magic web data plane API and microservices framework for Python developers, with a focus on reliability, correctness, and performance at scale.
https://falcon.readthedocs.io/en/stable/
Apache License 2.0
9.51k stars 937 forks source link

docs(user): add query strings tutorial #2239

Open bssyousefi opened 2 months ago

bssyousefi commented 2 months ago

Summary of Changes

Fill in the placeholder for Query Strings in the documentation. The placeholder only existed on WSGI tutorial, so the proposed change is just about that part of the documentation.

Related Issues

475

Pull Request Checklist

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once; it will save you a few review cycles!

If an item doesn't apply to your pull request, check it anyway to make it apparent that there's nothing to do.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

PR template inspired by the attrs project.

bssyousefi commented 2 months ago

I have a question regarding a part of the code.

 _IMAGE_NAME_PATTERN = re.compile(
            '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
        )

which I used from previous parts of the docs. the string used as the pattern will raise error in python. I guess we should use one of the following ways

 _IMAGE_NAME_PATTERN = re.compile(
            '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}.[a-z]{2,4}$'
        )
 _IMAGE_NAME_PATTERN = re.compile(
            r'[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
        )

However, I didn't try to fix it here and I thought that maybe I should first report it as an issue and then resolve it in another PR. Am I right or should I try the fix on this PR since it's a small change?

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (afe6c9f) to head (b49d829).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2239 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 63 63 Lines 7604 7604 Branches 1285 1285 ========================================= Hits 7604 7604 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bssyousefi commented 1 month ago

I have a question regarding a part of the code.

 _IMAGE_NAME_PATTERN = re.compile(
            '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
        )

which I used from previous parts of the docs. the string used as the pattern will raise error in python. I guess we should use one of the following ways

 _IMAGE_NAME_PATTERN = re.compile(
            '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}.[a-z]{2,4}$'
        )
 _IMAGE_NAME_PATTERN = re.compile(
            r'[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
        )

However, I didn't try to fix it here and I thought that maybe I should first report it as an issue and then resolve it in another PR. Am I right or should I try the fix on this PR since it's a small change?

@vytas7 Would you please also help me on this?

bssyousefi commented 1 week ago

@vytas7 I'm very sorry for the very long delay. I mistakenly thought that all the tests were passed and I didn't got time to get back to the project since I was a little bit busy. Thanks for you patience. I will be more available and responsible from now on.

vytas7 commented 1 week ago

No worries, @bssyousefi :slightly_smiling_face: We are now focusing on getting 4.0 out the door, so your PR is not urgent. But it will be nice to have it done when ready.

bssyousefi commented 1 week ago

I guess it's ready. I've updated the tests for the example look