build: Make 'prettier -w .' work from the root #15759
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
area: build
alexvy86
reviewed
Jun 1, 2023
…of unique columns in telemetry (microsoft#15827) ## Description Precursors: * PR microsoft#12863 moved the typings for our internal loggers to telemetry-utils package and added support for arrays. * PR microsoft#15667 upgraded the client release group to use the new types This PR updates that type further to add support for flat objects. This has become a more common practice to consolidate event-specific properties, e.g. in a `details` object. Today instrumentation does the JSON.stringify itself, but this is not ideal. It's proven the utility of logging properties like this, so this PR adds first-class support for this. ## Breaking Changes Yes, this is a breaking change. If a logger implements the existing logger contracts based on the existing type, it will not necessarily be equipped to handle a flat object. In practice, there shouldn't be any follow-up required. These types are only used for our internal logging contracts, which are always fulfilled by a class derived from `TelemetryLogger`, which actually handles this case just fine without any changes. The API surface we expect partners to use always speaks in `ITelemetryBaseLogger` which is unchanged here.
github-actions
bot
added
area: dds
github-actions
bot
added
area: contributor experience
area: examples
tylerbutler
changed the title
alexvy86
reviewed
Jun 5, 2023
package.json
Outdated
Comment on lines
61
to
62
| "prettier": "prettier --check . --ignore-path ./.prettierignore", | ||
| "prettier:fix": "prettier --write . --ignore-path ./.prettierignore", |
There was a problem hiding this comment.
This will this start formatting the server release group when run from the root, right? Is that intended? I think it shouldn't...
There was a problem hiding this comment.
Yes, it's intended. In the ideal case, this should never be a problem because CI enforces that everything is formatted properly already, so it shouldn't matter.
If we don't do it this way, where should the script to format the whole repo live?
There was a problem hiding this comment.
I think my honest answer to that is "here but server should live in a separate repo" 😆. Not strongly opposed to moving forward like this, just noting that my spidey sense reacted. One specific concern I have is if server folks update "their copy" of .prettierignore but no the root; we expect CI to fail in that case because it will format from the root and cause the validation for unexpected file changes to complain?
There was a problem hiding this comment.
I created new prettier:repo and format:repo scripts so the old ones still stay scoped to the client release group.
alexvy86
approved these changes
Jun 6, 2023
|
🔗 Found some broken links! 💔 Run a link check locally to find them. See linkcheck output |
connorskees
pushed a commit
to connorskees/FluidFramework
that referenced
this pull request
Jul 19, 2023
Related to microsoft#15758. This PR adds new prettier scripts to run on the whole repo. I also updated the prettierignore files to ignore additional files. The `prettier --write .` command should also now work from the root of any release group. --------- Co-authored-by: Scott Norton <scottnorton@microsoft.com> Co-authored-by: Mark Fields <markfields@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to #15758.
This PR adds new prettier scripts to run on the whole repo. I also updated the prettierignore files to ignore additional files. The
prettier --write .command should also now work from the root of any release group.