Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not modify generators files if nothing has changed (close #2895) #3412

Merged
merged 6 commits into from Aug 28, 2018

Conversation

Projects
None yet
4 participants
@jgsogo
Copy link
Member

commented Aug 28, 2018

  • Refer to the issue that supports this Pull Request: #2895
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • Add test

Add parameter only_if_modified to the utility function save to skip actual writing to disk if nothing has changed.

Implement this functionality in generators generated files.

Changelog: Fix: The generator files are only written in disk if the content of the generated file changes.

@jgsogo jgsogo requested review from lasote and memsharded Aug 28, 2018

@ghost ghost assigned jgsogo Aug 28, 2018

@ghost ghost added the stage: review label Aug 28, 2018

jgsogo added some commits Aug 28, 2018

@danimtb
Copy link
Member

left a comment

LGTM, just some tests

@@ -112,20 +112,28 @@ def save_append(path, content):
handle.write(to_file_bytes(content))


def save(path, content):
def save(path, content, only_if_modified=False):

This comment has been minimized.

Copy link
@danimtb

danimtb Aug 28, 2018

Member

As this is only used internally (tools.save() is not affected by this) and mostly for testing (I think). Would it be possible to benefit from a defaulted only_if_modified=True?

This comment has been minimized.

Copy link
@memsharded

memsharded Aug 28, 2018

Contributor

No, do not change the current default, dangerous. And I don't see the benefit, you mean performance? I doubt it, it will be slower.

@jgsogo

This comment has been minimized.

Copy link
Member Author

commented Aug 28, 2018

yes, I will add some for the save and save_files functions (and fix failing ones, of course)

@lasote

lasote approved these changes Aug 28, 2018

@memsharded memsharded merged commit e3c4aff into conan-io:develop Aug 28, 2018

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@memsharded memsharded added this to the 1.8 milestone Aug 28, 2018

@ghost ghost removed the stage: review label Aug 28, 2018

@jgsogo jgsogo deleted the jgsogo:issue/2895 branch Sep 19, 2018

grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018

Merge pull request conan-io#3412 from jgsogo/issue/2895
Do not modify generators files if nothing has changed (close conan-io#2895)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.