build(aio): improve ignored example file error message by petebacondarwin · Pull Request #19265 · angular/angular

petebacondarwin · 2017-09-19T09:17:43Z

This will provide a better error message for @wardbell in #18707 (comment)

petebacondarwin · 2017-09-19T09:19:25Z

Consider reviewing the changes to the spec files with whitespace changes ignored.

mary-poppins · 2017-09-19T09:25:59Z

You can preview b124ea4 at https://pr19265-b124ea4.ngbuilds.io/.

gkalpak · 2017-09-19T10:00:00Z

AFAICT, examplePaths will be relative, but filteredExamplePaths will be absolute.
So, this test won't work (and the current tests won't catch this).

I believe that you know wrong in this case.

I think the problem stems from the resolve mapping, right? I tested this by using yarn docs with changes to gitignore. I suspect that the resolve is effectively a noop.

I said "AFAICT" not "AFAIK" 😃 (I never know wrong 😛)

So, which part is wrong? exampleFiles being relative or filteredExampleFiles being absolute?
(I made my assumption based on this line, btw.)

Damn it you are right!

_{Sorry 😳}

The result was false positives, I was reporting examples as ignored when they were actually missing.

Actually as it turned out, it does indeed work as it is. The point is that if you get to calling the isExampleFiltered function then you already know that the file will not appear in the filteredExamplePaths list, so it doesn't matter that this will never match!

petebacondarwin · 2017-09-19T10:35:26Z

Hopefully this should be better

mary-poppins · 2017-09-19T10:44:21Z

You can preview 84cdc18 at https://pr19265-84cdc18.ngbuilds.io/.

mary-poppins · 2017-09-19T10:44:29Z

You can preview 8ccf5b7 at https://pr19265-8ccf5b7.ngbuilds.io/.

gkalpak

A couple of optional nitpicks.
LGTM otherwise 👍

gkalpak · 2017-09-19T10:51:03Z

Minor: This is a little confusing imo (as pre-filtered could mean that the paths were filtered in advance 😁).
Maybe Unfiltered or All?

gkalpak · 2017-09-19T10:51:24Z

gkalpak · 2017-09-19T10:57:57Z

Consider adding a comment that this method is expected to be called with paths that are not "filteredExamplePaths".

Addresses angular#18707 (comment)

petebacondarwin · 2017-09-19T11:44:11Z

I reworked it completely. PTAL @gkalpak

mary-poppins · 2017-09-19T11:52:27Z

You can preview b11f8e2 at https://pr19265-b11f8e2.ngbuilds.io/.

gkalpak

LGTM

gkalpak · 2017-09-19T15:41:20Z

+    },
+    /**
+     * Call this method to find out if an example was ignored.
+     * @param path a relative path to the example file to test for being ignored.


Might be worth mentioning that the path is relative to the examples folder.

Addresses #18707 (comment) PR Close #19265

petebacondarwin · 2017-09-20T21:11:19Z

Looking for a cherry-pick to stable too @IgorMinar

PR Close angular#19265

…19265) Addresses angular#18707 (comment) PR Close angular#19265

petebacondarwin · 2017-09-21T08:33:09Z

Landed in stable as 9624fda

angular-automatic-lock-bot · 2019-09-12T17:50:44Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

build(aio): remove commented out code

87961d6

petebacondarwin added comp: aio action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 19, 2017

petebacondarwin requested a review from gkalpak September 19, 2017 09:17

googlebot added the cla: yes label Sep 19, 2017

petebacondarwin mentioned this pull request Sep 19, 2017

docs: update universal guide based on PR 17573 #18707

Closed

gkalpak suggested changes Sep 19, 2017

View reviewed changes

petebacondarwin force-pushed the aio-improve-missing-example-error-message branch from b124ea4 to 8ccf5b7 Compare September 19, 2017 10:35

petebacondarwin force-pushed the aio-improve-missing-example-error-message branch from 8ccf5b7 to 84cdc18 Compare September 19, 2017 10:36

gkalpak approved these changes Sep 19, 2017

View reviewed changes

gkalpak added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 19, 2017

build(aio): improve error message for ignored example files

b11f8e2

Addresses angular#18707 (comment)

petebacondarwin force-pushed the aio-improve-missing-example-error-message branch from 84cdc18 to b11f8e2 Compare September 19, 2017 11:43

gkalpak approved these changes Sep 19, 2017

View reviewed changes

wardbell mentioned this pull request Sep 19, 2017

docs: update aio README and author guide about doc build errors #19276

Closed

3 tasks

petebacondarwin added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Sep 20, 2017

IgorMinar added the target: patch This PR is targeted for the next patch release label Sep 20, 2017

IgorMinar pushed a commit that referenced this pull request Sep 20, 2017

build(aio): improve error message for ignored example files (#19265)

381e680

Addresses #18707 (comment) PR Close #19265

IgorMinar closed this in 988b9f8 Sep 20, 2017

IgorMinar pushed a commit to IgorMinar/angular that referenced this pull request Sep 21, 2017

build(aio): remove commented out code (angular#19265)

e19b6a8

PR Close angular#19265

IgorMinar pushed a commit to IgorMinar/angular that referenced this pull request Sep 21, 2017

build(aio): improve error message for ignored example files (angular#…

9624fda

…19265) Addresses angular#18707 (comment) PR Close angular#19265

petebacondarwin reopened this Sep 21, 2017

petebacondarwin closed this Sep 21, 2017

petebacondarwin deleted the aio-improve-missing-example-error-message branch September 28, 2017 12:25

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 12, 2019

Conversation

petebacondarwin commented Sep 19, 2017

Uh oh!

petebacondarwin commented Sep 19, 2017

Uh oh!

mary-poppins commented Sep 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gkalpak Sep 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Sep 19, 2017

Uh oh!

mary-poppins commented Sep 19, 2017

Uh oh!

mary-poppins commented Sep 19, 2017

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Sep 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mary-poppins commented Sep 19, 2017

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Sep 20, 2017

Uh oh!

petebacondarwin commented Sep 21, 2017

Uh oh!

angular-automatic-lock-bot Bot commented Sep 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gkalpak Sep 19, 2017 •

edited

Loading

petebacondarwin commented Sep 19, 2017 •

edited

Loading