Skip to content

Conversation

@AlexV525
Copy link
Member

@AlexV525 AlexV525 commented Aug 16, 2023

https://github.com/flutter/tools_metadata is now configured as a submodule under resources/flutter instead of copy it manually.

The PR is huge because it deletes copied files. Be careful when reviewing the "Files changed" tab. 😃

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • 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.

@AlexV525 AlexV525 marked this pull request as ready for review August 16, 2023 16:04
@AlexV525 AlexV525 force-pushed the feat/resources-from-submodule branch from 961deec to 9804524 Compare August 17, 2023 04:40
@AlexV525

This comment was marked as outdated.

@AlexV525 AlexV525 force-pushed the feat/resources-from-submodule branch from a774072 to 9d6163a Compare August 17, 2023 07:59

export JAVA_HOME=`pwd`/../jdk-17.0.4.1.jdk/Contents/Home
export PATH=$PATH:$JAVA_HOME/bin
export JAVA_OPTS=" -Djava.net.preferIPv4Stack=false -Djava.net.preferIPv6Addresses=true"
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing this helps Kokoro to build successfully.

@AlexV525
Copy link
Member Author

Updating docs references since flutter.dev just went down for a few minutes and exposed the issue.

@AlexV525
Copy link
Member Author

AlexV525 commented Nov 1, 2023

cc @jwren @helin24 could you review this?

@AlexV525 AlexV525 requested review from helin24 and jwren November 1, 2023 10:30
@AlexV525
Copy link
Member Author

AlexV525 commented Nov 1, 2023

Also @DanTup

@AlexV525 AlexV525 requested a review from DanTup November 1, 2023 10:30
@DanTup
Copy link
Contributor

DanTup commented Nov 1, 2023

I don't know enough about this repo to review it so I'll defer to Jaime/Helin. The idea seems reasonable to me.

One question I'd have is about how this impacts the production builds of the plugin - will it automatically include these files in their new location (or is there some list somewhere of what gets included in the shipped plugin)? If it's automatically all included, will it include the whole tools_metadata repo (including things like the tool folder)? It's probably not a big deal (since I think it's just some additional text files), but perhaps something to be aware of.

@AlexV525
Copy link
Member Author

AlexV525 commented Nov 1, 2023

One question I'd have is about how this impacts the production builds of the plugin - will it automatically include these files in their new location (or is there some list somewhere of what gets included in the shipped plugin)? If it's automatically all included, will it include the whole tools_metadata repo (including things like the tool folder)?

Previous After
tools_metadata/resources tools_metadata

The resources/flutter will include the entire tools_metadata starting from this change. I didn't dig into whether irrelevant files (resources/flutter/!resources) could affect production builds though.

@jwren
Copy link
Member

jwren commented Jan 17, 2024

Thanks @AlexV525 for this PR. With less individuals working on the Flutter Plugin, and with many recent issues with the Flutter IJ build scripts, I have not had the time to verify that these changes won't have any negative consequences with the Gradle scripts, or other scripts we have that help maintain the Flutter Plugin for IntelliJ.

Leaving this PR open for the time being, thank you again for putting it together.

@AlexV525
Copy link
Member Author

@jwren Thanks!

With less individuals working on the Flutter Plugin, and with many recent issues with the Flutter IJ build scripts, I have not had the time to verify that these changes won't have any negative consequences with the Gradle scripts, or other scripts we have that help maintain the Flutter Plugin for IntelliJ.

I'd willing to help to verify if it works well if you can somehow provide any suggestions, and I'm fine to wait until you can figure out all fixes to build scripts. You can let me know once they are good to go.

@jwren
Copy link
Member

jwren commented Mar 27, 2024

Thank you for creating this PR, but for the time being I am closing it. PRs can't remain open for this long against the Flutter GitHub repo and with other concerns around the Flutter Plugin for IntelliJ I won't have the cycles to properly review and land this change.

@jwren jwren closed this Mar 27, 2024
@AlexV525 AlexV525 deleted the feat/resources-from-submodule branch April 10, 2025 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants