test: check that /examples has no changes #199

samschlegel · 2020-08-17T09:20:32Z

Fixes #198

This will fail until something like #191 gets merged to update the examples

UebelAndre · 2020-08-26T13:39:54Z

.travis.yml

@@ -40,3 +40,5 @@ script:
  - cargo install --debug -f --path .
  - cd ../
  - ./smoke-test.sh
+  # Check that /examples has no changes
+  - if [ -z "$(git status --porcelain)" ]; then echo "/examples is out of date. Please run `./smoke-test.sh` locally and commit the changes"; exit 1; fi


Doesn't this need to be '! -z`?

Suggested change

- if [ -z "$(git status --porcelain)" ]; then echo "/examples is out of date. Please run `./smoke-test.sh` locally and commit the changes"; exit 1; fi

- if [ ! -z "$(git status --porcelain)" ]; then echo "/examples is out of date. Please run `./smoke-test.sh` locally and commit the changes"; exit 1; fi

UebelAndre · 2020-08-26T13:55:45Z

@samschlegel I apologize for this but because of #202, master will generate some diffs in the examples. Could I get you to add the following diff to this PR?

diff --git a/impl/src/templates/remote_crates.bzl.template b/impl/src/templates/remote_crates.bzl.template
index ed0d9e8..df88ed5 100644
--- a/impl/src/templates/remote_crates.bzl.template
+++ b/impl/src/templates/remote_crates.bzl.template
@@ -17,8 +17,8 @@ def _new_git_repository(name, **kwargs):

 def {{workspace.gen_workspace_prefix}}_fetch_remote_crates():
 {%- if crates %}
-    {% for crate in crates %}
-        {%- if crate.source_details.git_data %}
+{% for crate in crates %}
+    {%- if crate.source_details.git_data %}
     _new_git_repository(
         name = "{{workspace.gen_workspace_prefix}}__{{crate.pkg_name | replace(from="-", to="_")}}__{{crate.pkg_version | slugify | replace(from="-", to="_")}}",
         remote = "{{crate.source_details.git_data.remote}}",
@@ -27,7 +27,7 @@ def {{workspace.gen_workspace_prefix}}_fetch_remote_crates():
         init_submodules = True,
         {%- include "templates/partials/remote_crates_patch.template" %}
     )
-        {%- else %}
+    {%- else %}
     _new_http_archive(
         name = "{{workspace.gen_workspace_prefix}}__{{crate.pkg_name | replace(from="-", to="_")}}__{{crate.pkg_version | slugify | replace(from="-", to="_")}}",
         url = "https://crates-io.s3-us-west-1.amazonaws.com/crates/{{crate.pkg_name}}/{{crate.pkg_name}}-{{crate.pkg_version}}.crate",
@@ -39,8 +39,8 @@ def {{workspace.gen_workspace_prefix}}_fetch_remote_crates():
         {%- include "templates/partials/remote_crates_patch.template" %}
         build_file = Label("{{workspace.workspace_path}}/remote:{{crate.pkg_name}}-{{crate.pkg_version}}.{{workspace.output_buildfile_suffix}}"),
     )
-        {%- endif %}
-    {% endfor %}
+    {%- endif %}
+{% endfor %}
 {%- else %}
     pass
 {% endif %}

I can create my own PR for this if you'd rather not. I just felt like it was a small enough change that it'd be worth making here instead of committing the diffs in the examples directory (which is only a whitespace difference)

samschlegel · 2020-08-26T16:45:16Z

I'm not really sure what happened with the test output here... it looks like it got mixed in with a bunch of output that I'm also not sure where came from since it happened after the smoke test exited 🤔

UebelAndre · 2020-08-26T16:47:34Z

~~@samschlegel Maybe you need to rebase?~~

edit: I misread, I see what you're saying now. But I still think your check will fail if you don't and do something about my comment above

samschlegel · 2020-08-26T16:48:18Z

I think it's because it was outputting to stdout and travis probably does something weird with stdout output at the end. Rebasing anyways, but don't think it's related

UebelAndre · 2020-08-26T16:51:51Z

@samschlegel Yeah, sounds likely. I have very little experience with Travis-CI though so I have to do some more digging to really help out. Hopefully your change fixes it though! 😄

samschlegel · 2020-08-26T16:53:53Z

Going to add a rust-toolchain file in this PR as well, since your changes in #185 use PathBuf capacity methods which weren't stabilized till 1.44 rust-lang/rust@7561714

…ion as Travis

@UebelAndre

Apparently my find iterates different than @UebelAndre's 😅

samschlegel · 2020-08-26T17:02:30Z

~~@samschlegel Maybe you need to rebase?~~

edit: I misread, I see what you're saying now. But I still think your check will fail if you don't and do something about my comment above

Yeah my intention is to make sure this fails correctly, then fix things and make sure it works there too. I'd like to be able to test this...but who CI's the CI‽

samschlegel · 2020-08-26T17:04:19Z

🤦 I'm using backticks in a double-quoted string lmaoooooo

samschlegel · 2020-08-26T17:07:27Z

andddd, that rebase did something WEIRD

google-cla · 2020-08-26T17:07:30Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

UebelAndre · 2020-08-26T17:28:00Z

@samschlegel Looks good but I just wanted to get clarity on this comment. Would you mind making that change as well? No worries if not, I'm just trying to cover up my shame 😅

UebelAndre

LGTM! 🚀

UebelAndre · 2020-08-26T17:55:30Z

@damienmg @dfreese Could someone with merge powers take a look at this pull-request?

damienmg

I just have a minor comment, there seems to be an unwanted edit in this PR.

impl/src/templates/remote_crates.bzl.template

This reverts commit 6ea6543.

damienmg · 2020-08-27T07:02:37Z

Thanks!

@generated

…r defects. (#207) This is a followup on comments made in #199 The `crates.bzl` file generated in the event that no crates are defined looks like the following ```python """ @generated cargo-raze crate workspace functions DO NOT EDIT! Replaced on runs of cargo-raze """ load("@bazel_tools//tools/build_defs/repo:git.bzl", "new_git_repository") # buildifier: disable=load load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") # buildifier: disable=load load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe") # buildifier: disable=load def raze_fetch_remote_crates(): """No crates were detected in the source Cargo.toml. This is a no-op""" pass ```

samschlegel mentioned this pull request Aug 18, 2020

Request: Update the examples directory #190

Closed

UebelAndre mentioned this pull request Aug 24, 2020

Updated examples #191

Merged

UebelAndre requested changes Aug 26, 2020

View reviewed changes

samschlegel added 4 commits August 26, 2020 09:47

test: check that /examples has no changes

b8a282c

Make error output a bit better?

86f7e05

fix test operator

0ae94af

Echo to stderr to hopefully not mix logs?

550c99d

samschlegel added 3 commits August 26, 2020 09:58

Add rust-toolchain file to make sure local builds track the same vers…

b07e46a

…ion as Travis

Sort examples in WORKSPACE

5d7a28f

Apparently my find iterates different than @UebelAndre's 😅

Update generated files

71779e4

don't accidentally run the smoke test again 🤦‍♂️

c79095e

samschlegel force-pushed the samschlegel/check-that-examples-has-no-changes branch from 384fa41 to c79095e Compare August 26, 2020 17:08

Add a fix for @UebelAndre to hide their shame 🤭

6ea6543

UebelAndre approved these changes Aug 26, 2020

View reviewed changes

damienmg approved these changes Aug 26, 2020

View reviewed changes

impl/src/templates/remote_crates.bzl.template Outdated Show resolved Hide resolved

damienmg reviewed Aug 26, 2020

View reviewed changes

impl/src/templates/remote_crates.bzl.template Outdated Show resolved Hide resolved

impl/src/templates/remote_crates.bzl.template Outdated Show resolved Hide resolved

Revert "Add a fix for @UebelAndre to hide their shame 🤭"

476c585

This reverts commit 6ea6543.

UebelAndre mentioned this pull request Aug 26, 2020

Updated remote crates.bzl template to use 'maybe' and fixed Buildifier defects. #207

Merged

damienmg merged commit 7b63f24 into google:master Aug 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: check that /examples has no changes #199

test: check that /examples has no changes #199

samschlegel commented Aug 17, 2020 •

edited

UebelAndre Aug 26, 2020

UebelAndre commented Aug 26, 2020

samschlegel commented Aug 26, 2020

UebelAndre commented Aug 26, 2020 •

edited

samschlegel commented Aug 26, 2020

UebelAndre commented Aug 26, 2020

samschlegel commented Aug 26, 2020 •

edited

samschlegel commented Aug 26, 2020

samschlegel commented Aug 26, 2020

samschlegel commented Aug 26, 2020

google-cla bot commented Aug 26, 2020

UebelAndre commented Aug 26, 2020

UebelAndre left a comment

UebelAndre commented Aug 26, 2020

damienmg left a comment

damienmg commented Aug 27, 2020

	- if [ -z "$(git status --porcelain)" ]; then echo "/examples is out of date. Please run `./smoke-test.sh` locally and commit the changes"; exit 1; fi
	- if [ ! -z "$(git status --porcelain)" ]; then echo "/examples is out of date. Please run `./smoke-test.sh` locally and commit the changes"; exit 1; fi

test: check that /examples has no changes #199

test: check that /examples has no changes #199

Conversation

samschlegel commented Aug 17, 2020 • edited

UebelAndre Aug 26, 2020

Choose a reason for hiding this comment

UebelAndre commented Aug 26, 2020

samschlegel commented Aug 26, 2020

UebelAndre commented Aug 26, 2020 • edited

samschlegel commented Aug 26, 2020

UebelAndre commented Aug 26, 2020

samschlegel commented Aug 26, 2020 • edited

samschlegel commented Aug 26, 2020

samschlegel commented Aug 26, 2020

samschlegel commented Aug 26, 2020

google-cla bot commented Aug 26, 2020

UebelAndre commented Aug 26, 2020

UebelAndre left a comment

Choose a reason for hiding this comment

UebelAndre commented Aug 26, 2020

damienmg left a comment

Choose a reason for hiding this comment

damienmg commented Aug 27, 2020

samschlegel commented Aug 17, 2020 •

edited

UebelAndre commented Aug 26, 2020 •

edited

samschlegel commented Aug 26, 2020 •

edited