Skip to content

Angular doc-gen reboot#14097

Closed
petebacondarwin wants to merge 5 commits into
angular:masterfrom
petebacondarwin:new-doc-gen
Closed

Angular doc-gen reboot#14097
petebacondarwin wants to merge 5 commits into
angular:masterfrom
petebacondarwin:new-doc-gen

Conversation

@petebacondarwin

@petebacondarwin petebacondarwin commented Jan 25, 2017

Copy link
Copy Markdown
Contributor

No description provided.

IgorMinar
IgorMinar approved these changes Jan 25, 2017
View reviewed changes

@IgorMinar IgorMinar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

otherwise lgtm. let's merge this and we can move things later if we need to. let's keep /angular.io directory just for the app.

Comment thread circle.yml Outdated

@IgorMinar IgorMinar Jan 25, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this should likely be part of travis and not circle, circle is used just for linting

@IgorMinar IgorMinar added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews pr_state: LGTM and removed action: discuss type: chore labels Jan 25, 2017
@IgorMinar

IgorMinar commented Jan 25, 2017

Copy link
Copy Markdown
Contributor

one more thing - we no longer use "chore" scopes. Please use "build(aio)" instead. More info: #14098

@petebacondarwin petebacondarwin force-pushed the new-doc-gen branch 7 times, most recently from 9752378 to 8120417 Compare January 25, 2017 23:24
@IgorMinar IgorMinar changed the title Angular doc-gen reboot (WIP) Angular doc-gen reboot Jan 26, 2017
@IgorMinar

IgorMinar commented Jan 26, 2017

Copy link
Copy Markdown
Contributor

@petebacondarwin is this good to go? if so, please remove the cleanup and wip labels and add "pr: merge" label. thanks. I also suggest you rebase.

@petebacondarwin 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 state: WIP labels Jan 26, 2017
@petebacondarwin

petebacondarwin commented Jan 26, 2017

Copy link
Copy Markdown
Contributor Author

rebased and ready to merge

gkalpak
gkalpak reviewed Jan 26, 2017
View reviewed changes

@gkalpak gkalpak left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't the docs app files live under the angular.io/ directory?

Comment thread docs/src/app/search-worker-client.ts Outdated

@gkalpak gkalpak Jan 26, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: Could possibly be simlified as:

return Observable.fromPromise(this.ready)

@petebacondarwin petebacondarwin Jan 26, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My RxJS foo is still growing but I think you are right if you mean:

  search(query: string) {
    return Observable.fromPromise(this.ready)
            .switchMap(() => this._createQuery(query));
  }

Comment thread docs/src/app/search-worker-client.ts Outdated

@gkalpak gkalpak Jan 26, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this (and similar statements) left intentionally?

@petebacondarwin petebacondarwin Jan 26, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

as I said, it was POC ... that is merged into angular/angular master!!

Comment thread docs/src/app/search-worker.js Outdated

@gkalpak gkalpak Jan 26, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the the --> the 😱

@petebacondarwin
docs(router): remove invalid jsdoc tags
df527b4 
The `@selector` tags are not valid.
Dgeni should be able to extract this information
from the directive annotation metadata.
@petebacondarwin
docs(xhr_backend): vanilla links do not need link tags
c31f22c
@petebacondarwin

petebacondarwin commented Jan 26, 2017

Copy link
Copy Markdown
Contributor Author

I have fixed up @gkalpak's comments and it is now again ready for merge

IgorMinar pushed a commit that referenced this pull request Jan 27, 2017
@petebacondarwin @IgorMinar
docs(router): remove invalid jsdoc tags (#14097)
e35c25d 
The `@selector` tags are not valid.
Dgeni should be able to extract this information
from the directive annotation metadata.
IgorMinar pushed a commit that referenced this pull request Jan 27, 2017
@petebacondarwin @IgorMinar
docs(http): vanilla links do not need link tags (#14097)
2dd9654
IgorMinar pushed a commit that referenced this pull request Jan 27, 2017
@petebacondarwin @IgorMinar
build(aio): update node dependencies for doc-gen (#14097)
d1d0ce7
IgorMinar pushed a commit that referenced this pull request Jan 27, 2017
@petebacondarwin @IgorMinar
build(aio): move doc-gen stuff from angular.io (#14097)
b776355
IgorMinar pushed a commit that referenced this pull request Jan 27, 2017
@petebacondarwin @IgorMinar
ci(aio): run docs-tests in travis (#14097)
b141a22
@IgorMinar

IgorMinar commented Jan 27, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

landed as 1e729d7...b141a22 - I rewrote a few commit messages to follow the new guidelines (the automated check is temporarily disabled so the PR didn't fail)

@IgorMinar IgorMinar closed this Jan 27, 2017
mhevery pushed a commit that referenced this pull request Feb 2, 2017
@petebacondarwin @mhevery
docs(router): remove invalid jsdoc tags (#14097)
3d1e536 
The `@selector` tags are not valid.
Dgeni should be able to extract this information
from the directive annotation metadata.
mhevery pushed a commit that referenced this pull request Feb 2, 2017
@petebacondarwin @mhevery
docs(http): vanilla links do not need link tags (#14097)
23bd0fb
@petebacondarwin petebacondarwin deleted the new-doc-gen branch March 21, 2017 12:00
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@petebacondarwin @juleskremer
docs(router): remove invalid jsdoc tags (angular#14097)
98b4d56 
The `@selector` tags are not valid.
Dgeni should be able to extract this information
from the directive annotation metadata.
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@petebacondarwin @juleskremer
docs(http): vanilla links do not need link tags (angular#14097)
3fe7f32
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@petebacondarwin @juleskremer
build(aio): update node dependencies for doc-gen (angular#14097)
1bec339
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@petebacondarwin @juleskremer
build(aio): move doc-gen stuff from angular.io (angular#14097)
82e1cbe
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@petebacondarwin @juleskremer
ci(aio): run docs-tests in travis (angular#14097)
6bb9446
@angular-automatic-lock-bot

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

Copy link
Copy Markdown

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 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@gkalpak gkalpak gkalpak left review comments
@IgorMinar IgorMinar IgorMinar approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@petebacondarwin @IgorMinar @gkalpak @googlebot