Skip to content
This repository was archived by the owner on Jul 29, 2024. It is now read-only.

chore(test): move expected_conditions_spec.js off of the control flow #5006

Closed
wants to merge 9 commits into from
Closed

chore(test): move expected_conditions_spec.js off of the control flow #5006

wants to merge 9 commits into from

Conversation

CrispusDH
Copy link
Contributor

  • updateexpected_conditions_spec
  • I'm not sure what to do with cripts/test.js

@CrispusDH
Copy link
Contributor Author

@cnishina , could you take a look at this PR, please

@CrispusDH CrispusDH changed the title Update expected_conditions_spec chore(test): move lexpected_conditions_spec.js off of the control flow Nov 7, 2018
@CrispusDH CrispusDH changed the title chore(test): move lexpected_conditions_spec.js off of the control flow chore(test): move expected_conditions_spec.js off of the control flow Nov 7, 2018
Copy link
Contributor

@cnishina cnishina left a comment

Choose a reason for hiding this comment

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

There are a lot of similar issues. Should be a pretty easy fix.

@@ -61,7 +61,7 @@ var executor = new Executor();

passingTests.forEach(function(passing_test) {
executor.addCommandlineTest(passing_test)
.assertExitCodeOnly();
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird spacing issue. Let's revert this part for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tried copy paste from origin but still has this weird space. I think it could be tab space thing. Maybe later I will make PR with ESLint that will be responsible with that kind of things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you want I can create a new PR where it issue will not be

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that 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 here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

1 similar comment
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that 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 here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

@googlebot googlebot added cla: no and removed cla: yes labels Nov 8, 2018
@cnishina
Copy link
Contributor

cnishina commented Nov 8, 2018

Doh. I deleted an entire file. :( Hrrrmmm...

@cnishina
Copy link
Contributor

cnishina commented Nov 8, 2018

Hmmm.... I also appear to have affected the cla. Darn it. Could you close this PR, open up a new one and try to not add a space in that test.js file? Thanks!

@cnishina
Copy link
Contributor

cnishina commented Nov 8, 2018

Yeah, other than that, the files lgtm.

@CrispusDH CrispusDH closed this Nov 8, 2018
@CrispusDH CrispusDH deleted the expected_conditions_spec branch November 8, 2018 09:42
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.

3 participants