Skip to content

Add snippet updater#4559

Closed
xiangyan99 wants to merge 3 commits intomainfrom
python_snippet_updater
Closed

Add snippet updater#4559
xiangyan99 wants to merge 3 commits intomainfrom
python_snippet_updater

Conversation

@xiangyan99
Copy link
Member

No description provided.

@xiangyan99 xiangyan99 requested a review from scbedd October 28, 2022 21:45
name = s[0].strip()
snippet = s[1]
# Remove extra spaces
spaces = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really appreciate a little code comment or something that maps out what is actually happening here. I am not convinced of this code as-is just from reading it.

A snippet is the complete code from the actual python file. So ripped directly from the python file it'll look like.

        # [START trio]
        from azure.core.pipeline.transport import TrioRequestsTransport

        async with AsyncPipeline(TrioRequestsTransport(), policies=policies) as pipeline:
            return await pipeline.run(request)
        # [END trio]

And we need to remove spaces, as we will be emplacing the snippet into the markdown file.

First question, why do we start with the second character of the snippet? Shouldn't we start with the first?

        if snippet[-1] == "\n":
            snippet = snippet[:-1]

I seriously do not get it.

Copy link
Member

@scbedd scbedd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm too dumb to track what the code is actually doing with the indexes, needs some explanation. The test additions etc are great and I'll approve once I understand the replacement.

@xiangyan99
Copy link
Member Author

Close the PR and use Azure/azure-sdk-for-python#26899 to track the code change.

@xiangyan99 xiangyan99 closed this Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants