X Tutup
The Wayback Machine - https://web.archive.org/web/20200627234353/https://github.com/angular/angular/pull/37026
Skip to content

/ angular

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: correct outdated dev instructions for public api golds #37026

Closed
wants to merge 1 commit into from
Closed

docs: correct outdated dev instructions for public api golds #37026

wants to merge 1 commit into from
+29 −24

Conversation

@ajitsinghkaler
Copy link
Contributor

ajitsinghkaler commented May 9, 2020

The public api .d.ts files were moved from tools/public_api_guard to goldens/public-api. and and a script
was added in #35768 which assist in testing the current state of the repo against the goldens as
well as a command for accepting all changes to the goldens in a single
command.

All these changes were not documented added documentation for new script command and an example of the new logs generated.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

No documentation on change of commit files directory

Issue Number: N/A

What is the new behavior?

Documentation added on change of commit files directory

Does this PR introduce a breaking change?

  • Yes
  • No

Other information
@googlebot googlebot added the cla: yes label May 9, 2020
@pullapprove pullapprove bot requested a review from kara May 9, 2020
@mhevery mhevery added the comp: docs label May 11, 2020
@ngbot ngbot bot added this to the needsTriage milestone May 11, 2020
@ajitsinghkaler
Copy link
Contributor Author

ajitsinghkaler commented Jun 6, 2020

@kara @pkozlowski-opensource @IgorMinar can you please look into this
@pullapprove pullapprove bot requested a review from jelbourn Jun 8, 2020
@ajitsinghkaler
Copy link
Contributor Author

ajitsinghkaler commented Jun 11, 2020

@jelbourn can you please review this
@ajitsinghkaler
Copy link
Contributor Author

ajitsinghkaler commented Jun 14, 2020

@jelbourn @kara can you please review this
@jelbourn
jelbourn reviewed Jun 17, 2020
View changes
Copy link
Member

jelbourn left a comment

I would change the commit message to something like:

docs: correct outdated dev instructions for public api golds

This change updates the dev instructions to reflect the location and generation of public API golds, which changed in #35768.

Also, @ajitsinghkaler, FWIW, adding additional mentions to a PR doesn't really help get reviews any faster, it just adds noise into the notification stream. We love getting community PRs, but the team's overall review load is super high at the moment- we're all trying to get to them as fast as we can.
docs/PUBLIC_API.md Outdated
@@ -62,31 +62,36 @@ Using yarn ensures that you are running the correct version of Bazel.
Here is an example of a Circle CI test failure that resulted from adding a new allowed type to a public property in `forms.d.ts`. Error messages from the API guard use [`git-diff` formatting](https://git-scm.com/docs/git-diff#_combined_diff_format).

This comment has been minimized.

Sign in to view
Copy link
@jelbourn

jelbourn Jun 17, 2020

Member

Should this say core.d.ts instead of forms.d.ts?

This comment has been minimized.

Sign in to view
Copy link
@ajitsinghkaler

ajitsinghkaler Jun 17, 2020

Author Contributor

Thanks for the review. I'll make the changes and will try not ping anymore.

Thanks for all the work you do.
@kara kara removed their request for review Jun 17, 2020
@ajitsinghkaler
docs: correct outdated dev instructions for public api golds
Loading status checks…
b531a5c 
This change updates the dev instructions to reflect the location and generation of public API golds, which changed in #35768.
@ajitsinghkaler ajitsinghkaler force-pushed the ajitsinghkaler:patch-3 branch from 71f4d36 to b531a5c Jun 19, 2020
@ajitsinghkaler ajitsinghkaler requested a review from jelbourn Jun 19, 2020
@ajitsinghkaler ajitsinghkaler changed the title docs: document change of file directory after #35768 & #36630 docs: correct outdated dev instructions for public api golds Jun 19, 2020
@jelbourn
jelbourn approved these changes Jun 19, 2020
View changes
Copy link
Member

jelbourn left a comment

LGTM, thanks!

Reviewed-for: public-api
@jelbourn
jelbourn approved these changes Jun 19, 2020
View changes
Copy link
Member

jelbourn left a comment

Reviewed-for: global-docs-approvers
@jelbourn
jelbourn approved these changes Jun 19, 2020
View changes
Copy link
Member

jelbourn left a comment

(one of these will do it...)

Reviewed-for: global-approvers
@jelbourn
jelbourn approved these changes Jun 19, 2020
View changes
Copy link
Member

jelbourn left a comment

Reviewed-for: global-docs-approvers
@jelbourn jelbourn requested a review from aikidave Jun 19, 2020
@jelbourn
Copy link
Member

jelbourn commented Jun 19, 2020

@aikidave I think this needs a global docs approver (I'm still confused about how this pullapprove setup works). This PR updates developer instructions for people writing code on the repo to fix an outdated directory, command to run, and example output.
@ajitsinghkaler
Copy link
Contributor Author

ajitsinghkaler commented Jun 26, 2020

@jelbourn pullapprove is already approved
@jelbourn
Copy link
Member

jelbourn commented Jun 26, 2020

Guess there was just some lag with the bot when I did it
AndrewKushnir added a commit that referenced this pull request Jun 26, 2020
@ajitsinghkaler @AndrewKushnir
docs: correct outdated dev instructions for public api golds (#37026)
Loading status checks…
5e287f6 
This change updates the dev instructions to reflect the location and generation of public API golds, which changed in #35768.

PR Close #37026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jelbourn jelbourn

@aikidave aikidave

Assignees
No one assigned
Labels
PR action: merge PR target: master & patch cla: yes comp: docs
Projects
None yet
Milestone
  needsTriage
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@ajitsinghkaler @jelbourn @mhevery @googlebot
You can’t perform that action at this time.
X Tutup