Skip to content

docs(docs-infra): Add dev-mode only mention for core/global#57365

Closed
JeanMeche wants to merge 1 commit intoangular:mainfrom
JeanMeche:coreglobal/devmode
Closed

docs(docs-infra): Add dev-mode only mention for core/global#57365
JeanMeche wants to merge 1 commit intoangular:mainfrom
JeanMeche:coreglobal/devmode

Conversation

@JeanMeche
Copy link
Member

@JeanMeche JeanMeche commented Aug 13, 2024

No description provided.

@angular-robot angular-robot bot added area: docs Related to the documentation area: docs-infra Angular.dev application and infrastructure labels Aug 13, 2024
@ngbot ngbot bot added this to the Backlog milestone Aug 13, 2024
@JeanMeche JeanMeche force-pushed the coreglobal/devmode branch 2 times, most recently from 4bbcf3a to 8797d03 Compare August 13, 2024 13:38
@github-actions
Copy link

github-actions bot commented Aug 13, 2024 •
edited
Loading

Deployed adev-preview for 913fae2 to: https://ng-dev-previews-fw--pr-angular-angular-57365-adev-prev-2wazrqi5.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

@JeanMeche JeanMeche marked this pull request as ready for review August 14, 2024 08:42
@pullapprove pullapprove bot requested a review from devversion August 14, 2024 08:42
devversion
devversion approved these changes Aug 14, 2024
View reviewed changes
packages/core/global/BUILD.bazel Outdated
Copy link
Member

@devversion devversion Aug 14, 2024

Choose a reason for hiding this comment

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

I would personally prefer window.ng globals, but I'm leaving to others.

Copy link
Member

@devversion devversion Aug 14, 2024

Choose a reason for hiding this comment

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

Also, it's a little unfortunate that the attribute is module_name here :( but might be fine for now

Copy link
Member Author

@JeanMeche JeanMeche Aug 14, 2024

Choose a reason for hiding this comment

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

I introduced a module_label argument which make the change a bit larger but we're have a better result now !

@angular-robot angular-robot bot requested a review from devversion August 14, 2024 18:47
@JeanMeche JeanMeche force-pushed the coreglobal/devmode branch 3 times, most recently from 062fae8 to f8aa046 Compare August 14, 2024 21:18
@JeanMeche JeanMeche mentioned this pull request Aug 14, 2024
Closed
@JeanMeche JeanMeche force-pushed the coreglobal/devmode branch 3 times, most recently from def65ee to 44d7756 Compare August 15, 2024 18:33
devversion
devversion approved these changes Aug 24, 2024
View reviewed changes
adev/src/app/features/references/api-items-section/api-items-section.component.html Outdated
Copy link
Member

@devversion devversion Aug 24, 2024

Choose a reason for hiding this comment

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

is title a pre-sanitized/bypassed HTML string here? (to avoid runtime noop sanitization for perf maybe?)

I could see us keeping it like that though; just to be "safe".

Copy link
Member

@devversion devversion Aug 26, 2024

Choose a reason for hiding this comment

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

I see this was resolved, but did you leave any comment here?

Copy link
Member Author

@JeanMeche JeanMeche Aug 26, 2024

Choose a reason for hiding this comment

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

Yes I added a comment. It only appears on the full diff

@angular-robot angular-robot bot requested a review from devversion August 24, 2024 09:22
@JeanMeche JeanMeche force-pushed the coreglobal/devmode branch 3 times, most recently from 7a365d6 to ad8baf6 Compare August 26, 2024 13:35
devversion
devversion approved these changes Aug 26, 2024
View reviewed changes
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM, minor comments, and the sanitization comment

adev/shared-docs/pipeline/api-gen/extraction/extract_api_to_json.bzl Outdated
Copy link
Member

@devversion devversion Aug 26, 2024

Choose a reason for hiding this comment

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

Maybe add a better description where this is commonly used + not sure if "JS" makes sense here?

adev/src/app/features/references/api-items-section/api-items-section.component.html Outdated
Copy link
Member

@devversion devversion Aug 26, 2024

Choose a reason for hiding this comment

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

I see this was resolved, but did you leave any comment here?

@angular-robot angular-robot bot requested a review from devversion August 26, 2024 14:31
@JeanMeche JeanMeche added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Aug 26, 2024
@angular-robot angular-robot bot requested a review from devversion August 28, 2024 11:51
@JeanMeche JeanMeche removed the action: merge The PR is ready for merge by the caretaker label Aug 28, 2024
@JeanMeche JeanMeche added the action: merge The PR is ready for merge by the caretaker label Aug 28, 2024
@AndrewKushnir AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Aug 29, 2024
@AndrewKushnir
Copy link
Contributor

AndrewKushnir commented Aug 29, 2024

Presubmit.

@AndrewKushnir
Copy link
Contributor

AndrewKushnir commented Aug 29, 2024

This PR was merged into the repository by commit 6c2beba.

The changes were merged into the following branches: main, 18.2.x

AndrewKushnir pushed a commit that referenced this pull request Aug 29, 2024
@JeanMeche @AndrewKushnir
docs(docs-infra): Add dev-mode only mention for core/global (#57365)
93bdbbc 
PR Close #57365
@AndrewKushnir AndrewKushnir removed the action: presubmit The PR is in need of a google3 presubmit label Aug 29, 2024
@JeanMeche JeanMeche mentioned this pull request Aug 29, 2024
Closed
@JeanMeche JeanMeche deleted the coreglobal/devmode branch August 30, 2024 00:01
@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Sep 30, 2024

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

Reviewers

@devversion devversion devversion approved these changes

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker adev: preview area: docs Related to the documentation area: docs-infra Angular.dev application and infrastructure target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

3 participants

@JeanMeche @AndrewKushnir @devversion