style: Spelling fixes #15323

jsoref · 2016-10-27T23:41:08Z

I understand that you want the commit to eventually have style as the type, I'm happy to roll up this series as a single commit w/ that as the main line.

For now, I'm more interested in feedback on the content of my commits.

googlebot · 2016-10-27T23:41:22Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

googlebot · 2016-10-27T23:41:23Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

jsoref · 2016-10-27T23:51:13Z

docs/content/guide/external-resources.ngdoc

@@ -137,7 +137,7 @@ You can find a larger list of Angular external libraries at [ngmodules.org](http
  [CodeAcademy](http://www.codecademy.com/courses/javascript-advanced-en-2hJ3J/0/1),
  [CodeSchool](https://www.codeschool.com/courses/shaping-up-with-angular-js)
 * **Paid online:**
-  [Pluralsite (3 courses)](http://www.pluralsight.com/training/Courses/Find?highlight=true&searchTerm=angularjs),
+  [Pluralsight (3 courses)](http://www.pluralsight.com/training/Courses/Find?highlight=true&searchTerm=angularjs),


I can't imagine someone with a domain of FooX calling themselves FooY...

jsoref · 2016-10-27T23:51:59Z

gdocs.js

@@ -39,7 +39,7 @@ if (flag === '--login') {
 }

 function help() {
-  console.log('Synopsys');
+  console.log('Synopsis');


This is a functional change to the documentation system. If you need such changes split out, I can, but ...

Interesting. I don't think this file is used anymore.

For my reference, this led to #15325

jsoref · 2016-10-27T23:52:45Z

test/BinderSpec.js

@@ -357,7 +357,7 @@ describe('Binder', function() {
    });
  });

-  it('ShoulIgnoreVbNonBindable', inject(function($rootScope, $compile) {
+  it('ShouldIgnoreVbNonBindable', inject(function($rootScope, $compile) {


This is a minor change to the testsuite's categorization system. It's unlikely that it would break anything, although historical data about failures would be disconnected.

jsoref · 2016-10-27T23:53:32Z

test/auto/injectorSpec.js

@@ -192,7 +192,7 @@ describe('injector', function() {


    it('should create $inject', function() {
-      var extraParans = angular.noop;
+      var extraParams = angular.noop;


It took me a long time to try to decide if this should be extraParams or extraParens, the initial commit was not remotely helpful.

jsoref · 2016-10-27T23:55:38Z

test/ng/compileSpec.js

@@ -11935,13 +11935,13 @@ describe('$compile', function() {
      testReplaceElementCleanup({});
    });
    it('should clean data of elements removed for directive templateUrl', function() {
-      testReplaceElementCleanup({asyncTmeplate: true});
+      testReplaceElementCleanup({asyncTemplate: true});


This is a change to the functionality of the test.

Good catch. Since the tests pass, the intended behavior has not been broken sometime in the past.

jsoref · 2016-10-27T23:56:40Z

test/ng/directive/selectSpec.js

@@ -383,7 +383,7 @@ describe('select', function() {
        scope.robot = '';
        compile('<select ng-model="robot">' +
                  '<option ng-repeat="opt in dynamicOptions" value="{{opt.val}}">{{opt.display}}</option>' +
-                '</selec>');
+                '</select>');


This is a genuine bug.

I assume the browser will close this broken tag by itself.

jsoref · 2016-10-27T23:59:55Z

test/ng/qSpec.js

@@ -2113,7 +2113,7 @@ describe('q', function() {
      });


-      it('should log exceptions throw in a progressack and stop propagation, but shoud NOT reject ' +
+      it('should log exceptions thrown in a progressback and stop propagation, but should NOT reject ' +


This was the only instance of progressack, I assumed it was a typo for progressback. I could be wrong.

progressBack was intended, but progressCallback is probably clearer.

Should I correct all of the instances? That would be much clearer.

So, upon trying a basic pair of commits to for this (see below), it became apparent that src/ng/q.js uses errback and progressBack as the names of the functions. I don't feel comfortable changing either name in a cleanup PR like this one, so I'm going to settle on progressBack and leave errback for now.

If someone wants to replace errback and progressBack with errorCallback and progressCallback respectively, they can do that in a subsequent PR.

jsoref · 2016-10-28T00:00:33Z

test/ngScenario/mocks.js

@@ -12,7 +12,7 @@ angular.scenario.testing.MockAngular.prototype.reset = function() {
 };

 angular.scenario.testing.MockAngular.prototype.poll = function() {
-  this.log.push('$brower.poll()');
+  this.log.push('$browser.poll()');


This could be a significant change.

It's just the string, so it should be good.

jsoref · 2016-10-28T00:03:18Z

I signed it!

googlebot · 2016-10-28T00:03:22Z

CLAs look good, thanks!

googlebot · 2016-10-28T00:03:23Z

CLAs look good, thanks!

Narretz · 2016-10-28T08:46:17Z

Hi @jsoref thanks for the PR. Except for one small suggestion, the changes are good.
Can you please separate changes to code and docs/ descriptions and put them into a single commit each, i.e. one docs(*): fix typos and one test(*): fix some inconsequential typos

Narretz · 2016-10-28T10:25:43Z

Sorry if I was not clear, but what I want is this:
docs(*)

changes in documentation text
changes in code comments, .e.g. /** asdf **/ and //asdf
code changes that only fix typos in strings that do not affect code (e.g. console.log)
changes to test descriptions (it, describe)

test(*)

code changes in Spec files, e.g the unclosed select tag, asyncTemplate, i.e. changes that change the code (but don't change any test outcome)

jsoref · 2016-10-28T10:29:05Z

I don't understand https://travis-ci.org/angular/angular.js/jobs/171341229 at all. I've temporarily dropped the code changes to see if that makes Travis happy.

@Narretz: Ok, I'll re-roll the split. What you're asking for is reasonable.

* select * synopsis * params * template

Narretz · 2016-10-28T10:45:06Z

The Travis failure is unrelated to your changes. It just couldn't connect to Saucelabs.

* a * allows * angularytics * animate * architecting * asynchronously * attribute * back * browser * callback * component * delimited * dependencies * dynamically * empty * encoded * explicitly * expression * fails * guarantees * hierarchy * highlight * identifiers * immediately * infinite * initialized * inputting * instance * interprets * linking * location * misformed * numerically * occurring * overridden * overwritten * parameters * Pluralsight * precedence * primitive * properly * prototypically * representation * response * separately * separator * should * specifying * supported * template * thrown * transclude * transclusion * transitions * trigger * useful

jsoref · 2016-10-28T11:28:08Z

@Narretz, re: saucelabs, is it possible to make the error easier for humans to interpret?
The last build for 15301 (I'm intentionally not referencing it formally because these items aren't linked) also failed w/ that error, and it's really hard for a normal reader to quickly figure out what the error means.

The output is:

The command "./scripts/travis/before_build.sh" failed and exited with 5 during .

But nothing clearly hints at what a 5 is or how it relates to any prior output.
Someone (like me) could naively search for before_build and then look for 5s and they'd find the ones I found:

>> build/angular-animate.js minified into build/angular-animate.min.js
build/angular.js:4719: WARNING - Throw expression is not a minErr instance.
          throw err;
          ^
build/angular.js:8278: WARNING - Throw expression is not a minErr instance.
            throw errors;
            ^
build/angular.js:12322: WARNING - Throw expression is not a minErr instance.
            throw e;
            ^
build/angular.js:13942: WARNING - Throw expression is not a minErr instance.
        throw e;
        ^
build/angular.js:17985: WARNING - Throw expression is not a minErr instance.
            throw e;
            ^
0 error(s), 5 warning(s)

Which is why I attempted to "fix" the "error" the way I did -- even though I knew that my changes shouldn't cause any problems (since I was merely rearranging which commits had which changes, and a previous build had already passed with the same set of total changes).

Narretz · 2016-10-28T11:40:49Z

You are right, the errors are difficult to interpret without prior knowledge. I'm not sure if we could try to interpret these errors in our test script. Maybe we could also adds docs for contributors about this.

jsoref · 2016-10-28T11:48:26Z

I'd encourage you to add handling to the test script.

There's a limit to how much documentation someone is going to read. If you do add any documentation outside the test script, then the test script needs to have code which does "if error, spit out url pointing to docs about errors".

Consider the case of someone browsing PRs (as I did to see the other one), they aren't actively contributing and have no particular reason to read the contributing docs on the offhand chance that they might talk about errors involving Travis.

Narretz · 2016-10-29T15:52:59Z

Thanks @jsoref
I'll open an issue for better error messages in the build.

googlebot added the cla: no label Oct 27, 2016

jsoref force-pushed the spelling branch from c8a3e3d to d7c2d5c Compare October 27, 2016 23:44

jsoref commented Oct 28, 2016

View reviewed changes

googlebot added cla: yes and removed cla: no labels Oct 28, 2016

Narretz self-assigned this Oct 28, 2016

Narretz added the type: docs label Oct 28, 2016

Narretz added this to the Backlog milestone Oct 28, 2016

jsoref force-pushed the spelling branch 3 times, most recently from 2a3b563 to 4b01319 Compare October 28, 2016 10:22

test(*): fix some inconsequential typos

7a959a3

* select * synopsis * params * template

jsoref force-pushed the spelling branch 2 times, most recently from 2b8cd51 to c948506 Compare October 28, 2016 11:14

Narretz merged commit 5cce6e2 into angular:master Oct 29, 2016

Narretz mentioned this pull request Oct 29, 2016

Imrpove error logging on Travis #15335

Open

jsoref deleted the spelling branch October 30, 2016 23:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

style: Spelling fixes #15323

style: Spelling fixes #15323

jsoref commented Oct 27, 2016

googlebot commented Oct 27, 2016

googlebot commented Oct 27, 2016

jsoref Oct 27, 2016

jsoref Oct 27, 2016

Narretz Oct 28, 2016

jsoref Oct 28, 2016

jsoref Oct 27, 2016

jsoref Oct 27, 2016

jsoref Oct 27, 2016

Narretz Oct 28, 2016

jsoref Oct 27, 2016

Narretz Oct 28, 2016

jsoref Oct 28, 2016

jsoref Oct 27, 2016

Narretz Oct 28, 2016

jsoref Oct 28, 2016

jsoref Oct 28, 2016

jsoref Oct 28, 2016

Narretz Oct 28, 2016

jsoref commented Oct 28, 2016

googlebot commented Oct 28, 2016

googlebot commented Oct 28, 2016

Narretz commented Oct 28, 2016

Narretz commented Oct 28, 2016 •

edited

Loading

jsoref commented Oct 28, 2016

Narretz commented Oct 28, 2016

jsoref commented Oct 28, 2016

Narretz commented Oct 28, 2016

jsoref commented Oct 28, 2016

Narretz commented Oct 29, 2016

style: Spelling fixes #15323

style: Spelling fixes #15323

Conversation

jsoref commented Oct 27, 2016

googlebot commented Oct 27, 2016

googlebot commented Oct 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsoref commented Oct 28, 2016

googlebot commented Oct 28, 2016

googlebot commented Oct 28, 2016

Narretz commented Oct 28, 2016

Narretz commented Oct 28, 2016 • edited Loading

jsoref commented Oct 28, 2016

Narretz commented Oct 28, 2016

jsoref commented Oct 28, 2016

Narretz commented Oct 28, 2016

jsoref commented Oct 28, 2016

Narretz commented Oct 29, 2016

Narretz commented Oct 28, 2016 •

edited

Loading