Skip to content
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

[tool] Add note about clang version used in CI to tool readme #7273

Merged
merged 3 commits into from
Aug 2, 2024

Conversation

Rexios80
Copy link
Member

@Rexios80 Rexios80 commented Aug 1, 2024

This repo uses clang 15 to format files. Newer versions of clang format code differently. This PR adds a note to the tool README about this and adds sample instructions for how to install clang 15 on macOS. We might want to add instructions for other platforms.

Fixes flutter/flutter#152651

Additional context

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style], or this PR is [exempt from CHANGELOG changes].
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@Rexios80 Rexios80 requested a review from stuartmorgan August 1, 2024 14:40
@Rexios80
Copy link
Member Author

Rexios80 commented Aug 1, 2024

@stuartmorgan It appears that Fuchsia is using clang version 16.0.0 now. Do you think it is important to keep the version this repo uses in sync with that? Or should we leave it at 15.0.0?

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

I'm in favor of adding a note about what version is being used, but I'm not sure trying to maintain detailed instructions that work for everyone, using tools that we don't control, is a good idea. I think what's here is already probably not right for everyone, and by the time we fix that, and someday add other platforms, and then inevitably people want different package manager instructions on Linux, etc., this will be a significant burden to maintain.

script/tool/README.md Outdated Show resolved Hide resolved
script/tool/README.md Outdated Show resolved Hide resolved
@stuartmorgan
Copy link
Contributor

@stuartmorgan It appears that Fuchsia is using clang version 16.0.0 now. Do you think it is important to keep the version this repo uses in sync with that?

I can't think of any reason why they would need to match.

@Rexios80
Copy link
Member Author

Rexios80 commented Aug 1, 2024

I agree that maintaining installation instructions for clang is outside the scope of this repo. I removed the macOS example.

@Rexios80 Rexios80 changed the title [tool] Add note about clang version and sample guide for macOS [tool] Add note about clang version used in CI to tool readme Aug 1, 2024
@Rexios80 Rexios80 requested a review from stuartmorgan August 1, 2024 15:12
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM

@Rexios80 Rexios80 added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 2, 2024
@auto-submit auto-submit bot merged commit cc9ff47 into flutter:main Aug 2, 2024
76 checks passed
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 2, 2024
flutter/packages@27896d1...cc9ff47

2024-08-02 [email protected] [tool] Add note about clang version used in CI to tool readme (flutter/packages#7273)
2024-08-02 [email protected] [ci] Re-enable google_maps_flutter_web integration tests. (flutter/packages#7269)
2024-08-01 [email protected] [web] Update package:web to ">=0.5.1 <2.0.0" (flutter/packages#7202)
2024-08-01 [email protected] [go_router] Documentation for StatefulShellRoute (flutter/packages#6308)
2024-08-01 [email protected] [google_maps_flutter] Switch Android examples to TLHC (flutter/packages#7282)
2024-08-01 [email protected] Manual roll Flutter from f817e51 to e2e4398 (5 revisions) (flutter/packages#7276)
2024-08-01 [email protected] [various] Clean up C++ formatting (flutter/packages#7272)
2024-08-01 [email protected] [shared_preferences] fix stringlist bug (flutter/packages#7268)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TytaniumDev pushed a commit to TytaniumDev/flutter that referenced this pull request Aug 7, 2024
flutter/packages@27896d1...cc9ff47

2024-08-02 [email protected] [tool] Add note about clang version used in CI to tool readme (flutter/packages#7273)
2024-08-02 [email protected] [ci] Re-enable google_maps_flutter_web integration tests. (flutter/packages#7269)
2024-08-01 [email protected] [web] Update package:web to ">=0.5.1 <2.0.0" (flutter/packages#7202)
2024-08-01 [email protected] [go_router] Documentation for StatefulShellRoute (flutter/packages#6308)
2024-08-01 [email protected] [google_maps_flutter] Switch Android examples to TLHC (flutter/packages#7282)
2024-08-01 [email protected] Manual roll Flutter from f817e51 to e2e4398 (5 revisions) (flutter/packages#7276)
2024-08-01 [email protected] [various] Clean up C++ formatting (flutter/packages#7272)
2024-08-01 [email protected] [shared_preferences] fix stringlist bug (flutter/packages#7268)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
flutter/packages@27896d1...cc9ff47

2024-08-02 [email protected] [tool] Add note about clang version used in CI to tool readme (flutter/packages#7273)
2024-08-02 [email protected] [ci] Re-enable google_maps_flutter_web integration tests. (flutter/packages#7269)
2024-08-01 [email protected] [web] Update package:web to ">=0.5.1 <2.0.0" (flutter/packages#7202)
2024-08-01 [email protected] [go_router] Documentation for StatefulShellRoute (flutter/packages#6308)
2024-08-01 [email protected] [google_maps_flutter] Switch Android examples to TLHC (flutter/packages#7282)
2024-08-01 [email protected] Manual roll Flutter from f817e51 to e2e4398 (5 revisions) (flutter/packages#7276)
2024-08-01 [email protected] [various] Clean up C++ formatting (flutter/packages#7272)
2024-08-01 [email protected] [shared_preferences] fix stringlist bug (flutter/packages#7268)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[packages] Cleanup unformatted files
2 participants