X Tutup
Skip to content

fix(public_spec): check exports of barrels instead of angular2/angular2#5821

Closed
jeffbcross wants to merge 3 commits intoangular:masterfrom
jeffbcross:barrel-public-spec
Closed

fix(public_spec): check exports of barrels instead of angular2/angular2#5821
jeffbcross wants to merge 3 commits intoangular:masterfrom
jeffbcross:barrel-public-spec

Conversation

@jeffbcross
Copy link
Copy Markdown
Contributor

@jeffbcross jeffbcross commented Dec 11, 2015

This changes the public api spec to check each public barrel individually
to make sure its API has not changed. The previous API spec has been
preserved but split into respective barrels.

The compiler barrel has been added to the spec, along with all of its
public exports. Previously, angular2/angular2 was only exporting a
handful of symbols from compiler, so there are now many more symbols
being tested in the spec for compiler than previously.

Part of #5710

CC @IgorMinar

@jeffbcross jeffbcross added the action: review The PR is still awaiting reviews from at least one requested reviewer label Dec 11, 2015
@jeffbcross jeffbcross added this to the beta.0 milestone Dec 11, 2015
@jeffbcross jeffbcross mentioned this pull request Dec 11, 2015
Closed
10 tasks
@pkozlowski-opensource
Copy link
Copy Markdown
Member

pkozlowski-opensource commented Dec 11, 2015

@jeffbcross thnx for the work on this! It is kind of hard to review since GitHub doesn't show diff for the test (probably the diff gets too big for its taste...). From the brief look changes seem reasonable for me but:

  • TravisCI is not happy (Dart part) - looks like a legit failure
  • Would be good to have another look from one of the "Public API masters"

But yeh, I guess sorting out Dart needs to happen first

@pkozlowski-opensource pkozlowski-opensource added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Dec 11, 2015
@jeffbcross
Copy link
Copy Markdown
Contributor Author

jeffbcross commented Dec 11, 2015

@pkozlowski-opensource Dart is an easy fix (unless there are more problems beyond the initial obvious problem :))

Yeah, the diff is hard. Here's what I can tell you about my process to ensure that the list is correct.

  • Every API that was in NG_ALL is in one of the other lists; nothing was removed.
  • I looked at angular2.ts to determine which barrels should be included in this spec.
  • I tested each non-core barrel one at a time, and copy-pasted the missing parts from NG_CORE to NG_____. Then I finally tested NG_CORE and it was green.
  • For the compiler barrel, I moved the APIs that were exported from angular2.ts to the new compiler barrel list, ran the test, then added all the remaining compiler barrel exported APIs to the spec.

So nothing was removed, but compiler APIs were added.

I'll get one of the Guardians of the API™ to review as well.

@pkozlowski-opensource
Copy link
Copy Markdown
Member

pkozlowski-opensource commented Dec 11, 2015

Sounds good @jeffbcross 👍

@jeffbcross
Copy link
Copy Markdown
Contributor Author

jeffbcross commented Dec 11, 2015

Correction: Dart does have more problems :).

@pkozlowski-opensource pkozlowski-opensource mentioned this pull request Dec 11, 2015
Closed
@jeffbcross
fix(public_spec): check exports of barrels instead of angular2/angular2
53b9d26 
This changes the public api spec to check each public barrel individually
to make sure its API has not changed. The previous API spec has been
preserved but split into respective barrels.

The compiler barrel has been added to the spec, along with all of its
public exports. Previously, angular2/angular2 was only exporting a
handful of symbols from compiler, so there are now many more symbols
being tested in the spec for compiler than previously.

Part of angular#5710
jeffbcross
jeffbcross reviewed Dec 11, 2015
View reviewed changes
modules/angular2/test/symbol_inspector/symbol_inspector.dart Outdated
Copy link
Copy Markdown
Contributor Author

@jeffbcross jeffbcross Dec 11, 2015

Choose a reason for hiding this comment

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

Move this back

Copy link
Copy Markdown
Contributor Author

@jeffbcross jeffbcross Dec 11, 2015

Choose a reason for hiding this comment

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

Nvm

@tbosch tbosch added pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 11, 2015
@jeffbcross
Copy link
Copy Markdown
Contributor Author

jeffbcross commented Dec 11, 2015

Got verbal LGTM from @tbosch with latest changes, once I make minor fix to symbol_inspector.dart.

@pkozlowski-opensource FYI

@tbosch tbosch 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 Dec 11, 2015
@pkozlowski-opensource
Copy link
Copy Markdown
Member

pkozlowski-opensource commented Dec 11, 2015

Awesome! As soon as this one gets in we need to merge #5828 and see what happens :-)

@jeffbcross
fix(public_spec): check exports of barrels instead of angular2/angular2
44ff261 
This changes the public api spec to check each public barrel individually
to make sure its API has not changed. The previous API spec has been
preserved but split into respective barrels.

The compiler barrel has been added to the spec, along with all of its
public exports. Previously, angular2/angular2 was only exporting a
handful of symbols from compiler, so there are now many more symbols
being tested in the spec for compiler than previously.

Part of angular#5710
@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Dec 11, 2015

Merging PR #5821 on behalf of @vsavkin to branch presubmit-vsavkin-pr-5821.

@vsavkin vsavkin removed the action: merge The PR is ready for merge by the caretaker label Dec 11, 2015
@jeffbcross
Copy link
Copy Markdown
Contributor Author

jeffbcross commented Dec 11, 2015

@tbosch is taking over fixing symbol_inspector to get rid of the DartAnalyzer unused import errors.

@jeffbcross jeffbcross assigned tbosch and unassigned jeffbcross Dec 11, 2015
@jeffbcross jeffbcross added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Dec 11, 2015
@tbosch tbosch mentioned this pull request Dec 11, 2015
Closed
@tbosch tbosch closed this Dec 11, 2015
jeffbcross added a commit that referenced this pull request Dec 11, 2015
@jeffbcross @tbosch
fix(public_spec): check exports of barrels instead of angular2/angular2
979162d 
This changes the public api spec to check each public barrel individually
to make sure its API has not changed. The previous API spec has been
preserved but split into respective barrels.

The compiler barrel has been added to the spec, along with all of its
public exports. Previously, angular2/angular2 was only exporting a
handful of symbols from compiler, so there are now many more symbols
being tested in the spec for compiler than previously.

Part of #5710
Closes #5841
Supercedes #5821
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Sep 7, 2019

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews cla: yes

Projects

None yet

Milestone

beta.0

Development

Successfully merging this pull request may close these issues.

6 participants

@jeffbcross @pkozlowski-opensource @mary-poppins @vsavkin @tbosch @googlebot
X Tutup