Skip to content

Compare files manually. - #710

Merged
theacodes merged 1 commit into
masterfrom
storage
Dec 13, 2016
Merged

Compare files manually.#710
theacodes merged 1 commit into
masterfrom
storage

Conversation

@jerjou

Copy link
Copy Markdown
Contributor

Addresses #707

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

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

Copy link
Copy Markdown
ContributorAuthor

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

Copy link
Copy Markdown
Contributor

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

@jerjou
jerjouforce-pushed the storage branch 2 times, most recently from 382ef96 to 249a2f2CompareDecember 12, 2016 19:10
@jerjou

Copy link
Copy Markdown
ContributorAuthor

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

Comment threadstorage/api/crud_object.py Outdated

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?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ah. right.

@ryanmatsryanmats assigned theacodes and ryanmats and unassigned theacodes and ryanmatsDec 13, 2016
@theacodes
theacodes merged commit ca2d5fc into masterDec 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
* 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>
chalmerlowe pushed a commit that referenced this pull request Apr 7, 2026
* samples: add endpoint sample
* lint
* properly check endpoint from client
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jerjou@theacodes@googlebot@ryanmats