Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Compare files manually.#710

Merged
theacodes merged 1 commit into
masterGoogleCloudPlatform/python-docs-samples:masterfrom
storageGoogleCloudPlatform/python-docs-samples:storageCopy head branch name to clipboard
Dec 13, 2016
Merged

Compare files manually.#710
theacodes merged 1 commit into
masterGoogleCloudPlatform/python-docs-samples:masterfrom
storageGoogleCloudPlatform/python-docs-samples:storageCopy head branch name to clipboard

Conversation

@jerjou

@jerjou jerjou commented Dec 9, 2016

Copy link
Copy Markdown
Contributor

Addresses #707

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 9, 2016
@jerjou jerjou assigned jerjou, theacodes and ryanmats and unassigned jerjou Dec 9, 2016
@theacodes

theacodes commented Dec 9, 2016

Copy link
Copy Markdown
Contributor

Hmm, I liked the idea of just closing the handle to the named temporary file and then using filecmp.

@jerjou

jerjou commented Dec 9, 2016

Copy link
Copy Markdown
Contributor Author

Oh. It just seemed less clean. eg:

  • You're now manually in charge of file cleanup, which feels like it should be the context manager's job
  • The use of filecmp with a filename when you've already got a handle on the file always felt a bit hacky.
  • The file comparison is now both inside and outside of the context manager, which breaks it up visually even though it's the same conceptual operation

Granted, this is a bit more verbose and low-level, but I feel like it flows better.

@theacodes

theacodes commented Dec 12, 2016

Copy link
Copy Markdown
Contributor

Can we just remove the file comparison altogether from this sample?

@jerjou jerjou force-pushed the storage branch 2 times, most recently from 382ef96 to 249a2f2 Compare December 12, 2016 19:10
@jerjou

jerjou commented Dec 12, 2016

Copy link
Copy Markdown
Contributor Author

Oh yeah - sure. Apparently this file isn't even referenced in the docs anymore.
Done.

theacodes
theacodes reviewed Dec 12, 2016
View reviewed changes
Comment thread storage/api/crud_object.py Outdated

@theacodes theacodes Dec 12, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove this?

@jerjou jerjou Dec 12, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah. right.

@ryanmats ryanmats assigned theacodes and ryanmats and unassigned theacodes and ryanmats Dec 13, 2016
@theacodes theacodes merged commit ca2d5fc into master Dec 13, 2016
@theacodes theacodes deleted the storage branch December 13, 2016 18:15
Linchin added a commit that referenced this pull request Aug 18, 2025
@gcf-owl-bot @parthea @Linchin
chore(python): Add Python 3.12 (#710)
60745d7 
* chore(python): Add Python 3.12

Source-Link: googleapis/synthtool@af16e6d
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:bacc3af03bff793a03add584537b36b5644342931ad989e3ba1171d3bd5399f5

* Add python 3.12 to setup.py

* add dependency for 3.12

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* update owlbot

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* undo owlbot change

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Co-authored-by: Linchin <lingqing.gan@gmail.com>
@Linchin Linchin mentioned this pull request Aug 18, 2025
Merged
9 tasks
chalmerlowe pushed a commit that referenced this pull request Apr 7, 2026
samples: add endpoint sample (#710)
a73aca8 
* samples: add endpoint sample

* lint

* properly check endpoint from client
@chalmerlowe chalmerlowe mentioned this pull request Apr 7, 2026
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@theacodes theacodes theacodes left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@theacodes theacodes

@ryanmats ryanmats

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jerjou @theacodes @googlebot @ryanmats
Morty Proxy This is a proxified and sanitized view of the page, visit original site.