Skip to content

Conversation

@maiqbal11
Copy link
Contributor

@maiqbal11 maiqbal11 commented Jan 7, 2020

Needed to unblock #554. Contains the same logic as #528 which was reverted.

setup.py Outdated
Comment on lines 84 to 88
if os.path.exists(staging_root_dir):
shutil.rmtree(build_dir)

if os.path.exists(built_protos_dir):
shutil.rmtree(built_protos_dir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like these four lines can be combined together?

if os.path.exists(build_dir):
    shutil.rmtree(build_dir)

Am I missing something here?


proto_files = glob.glob(str(proto_src_dir / '**' / '*.proto'),
recursive=True)
shutil.copytree(proto_src_dir, staging_dir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't seems like staging_dir is used anywhere else. Maybe we can remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's being used to do the copying as you pointed here. In the actual build command, we are using the same directory structure without explicitly referencing the staging_dir. Might be useful to walk through this offline as it's slightly tedious to explain here.

@maiqbal11 maiqbal11 merged commit 0b68e47 into dev Jan 8, 2020
@maiqbal11 maiqbal11 deleted the update-proto branch January 8, 2020 19:08
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.

3 participants