Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Consolidate findmake #1426

Merged
merged 15 commits into from Sep 11, 2019
Merged

Conversation

tst-basilhuffman
Copy link
Collaborator

FindMake code added to AssetFactory.h. Templating errors, looking to fix

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@tst-basilhuffman tst-basilhuffman changed the title WIP: Consolidate findmake Consolidate findmake Sep 9, 2019
Copy link
Collaborator

@tst-lsavoie tst-lsavoie left a comment

Choose a reason for hiding this comment

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

Looks solid overall. The main thing is to fix the merge conflicts. I've also made some other small comments.

I'm not done testing yet but the tests I've run so far have passed.

@tst-lsavoie
Copy link
Collaborator

All commits in this PR are either by the author or were previously submitted to this repo by authors with signed CLAs, so there are not CLA issues.

Copy link
Collaborator

@tst-lsavoie tst-lsavoie left a comment

Choose a reason for hiding this comment

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

All of my important comments have been addressed and the code looks fine visually. I still need to finish running tests before I can approve.

Copy link
Collaborator

@tst-lsavoie tst-lsavoie left a comment

Choose a reason for hiding this comment

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

All tests passed. Everything looks good.

Copy link
Collaborator

@tst-sbayard tst-sbayard left a comment

Choose a reason for hiding this comment

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

Code looks good.

@tst-sbayard tst-sbayard merged commit 59fec66 into google:master Sep 11, 2019
husf-dsheremata added a commit to husf-dsheremata/earthenterprise that referenced this pull request Sep 16, 2019
* 'master' of https://github.com/google/earthenterprise: (51 commits)
  call Depends() for generated header files used by geautoingest (google#1448)
  Added prune call to the end of SaveDirtyToDotNew for clearing cache of items written to disk. (google#1446)
  Fixed tables in 3.2 release notes
  Generated html for the new rst Added title to answer/*.rst fixed formatting in new 5.3.2 release notes
  Update index.rst
  Consolidate findmake (google#1426)
  Add files via upload
  Add files via upload
  Consolidate make functions (google#1424)
  Optimize child state change handling (google#1423)
  Committing the generated html and makefile changes
  Added a step in Makefile to rename the _ folders after html generation
  Reorganized more folders
  sync with master changes
  honor JAVA_HOME
  Organizing Docs in to folders and renaming files from numbers to meaningful names
  Log when cache size is exceeded (google#1402)
  Test that the fatal log file is written as appropriate
  Override log file writing in unit tests
  Write fatal log files from a virtual function
  ...
@tst-basilhuffman tst-basilhuffman deleted the consolidate-findmake branch June 1, 2020 01:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants