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.

Sign up for GitHub

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

Jump to bottom

Change gen-class-name for fonts colors assets [PR-280](https://github.com/FlutterGen/flutter_gen/pull/280) #286

Merged
merged 1 commit into from Sep 12, 2022
Merged

Change gen-class-name for fonts colors assets [PR-280](https://github.com/FlutterGen/flutter_gen/pull/280) #286

merged 1 commit into from Sep 12, 2022

Conversation

Cu-Toof
Copy link
Contributor

@Cu-Toof Cu-Toof commented Sep 2, 2022

No description provided.

@Cu-Toof
Copy link
Contributor Author

Cu-Toof commented Sep 2, 2022 •
edited

@thangnc @wasabeef sorry, my mistake for closing PR-280.
I have fixed all @thangnc comments and created a new pr.
Please check it.
Thanks

@Cu-Toof Cu-Toof force-pushed the feature/change-gen-class-name-for-fonts-colors-assets branch from 51a24fb to d9ad059 Compare September 2, 2022 02:26
@Cu-Toof Cu-Toof changed the title Change gen-class-name for fonts colors assets Change gen-class-name for fonts colors assets [PR-280](https://github.com/FlutterGen/flutter_gen/pull/280) Sep 2, 2022
@Cu-Toof Cu-Toof changed the title Change gen-class-name for fonts colors assets [PR-280](https://github.com/FlutterGen/flutter_gen/pull/280) Change gen-class-name for fonts colors assets (PR-280)[https://github.com/FlutterGen/flutter_gen/pull/280] Sep 2, 2022
@Cu-Toof Cu-Toof changed the title Change gen-class-name for fonts colors assets (PR-280)[https://github.com/FlutterGen/flutter_gen/pull/280] Change gen-class-name for fonts colors assets [PR-280](https://github.com/FlutterGen/flutter_gen/pull/280) Sep 2, 2022
@Cu-Toof Cu-Toof force-pushed the feature/change-gen-class-name-for-fonts-colors-assets branch 2 times, most recently from 4bf74c8 to 1f106fa Compare September 4, 2022 05:02
@wasabeef wasabeef self-requested a review September 6, 2022 06:00
@wasabeef
Copy link
Member

wasabeef commented Sep 7, 2022

@Cu-Toof
Thank you for your PR. plase fix the conflicts.

thangnc
thangnc reviewed Sep 7, 2022
View reviewed changes
Copy link

@thangnc thangnc left a comment

Choose a reason for hiding this comment

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

Basically LGTM except should rebase on main branch. I left 1 minor comment

packages/core/test_resources/pubspec_assets_exclude_files.yaml Outdated
- assets/images/icons/paint.svg
- assets/images/icons/dart@test.svg
- assets/json/fruits.json
- pictures/chip5.jpg
Copy link

Choose a reason for hiding this comment

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

Please add 1 new line for EOF here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thangnc I have updated PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thangnc Please review my PR
If everything is ok, please notify me and @wasabeef Could you merge my PR?

@Cu-Toof Cu-Toof force-pushed the feature/change-gen-class-name-for-fonts-colors-assets branch from 1f106fa to 655ef51 Compare September 10, 2022 02:28
thangnc
thangnc approved these changes Sep 10, 2022
View reviewed changes
Copy link

@thangnc thangnc left a comment

Choose a reason for hiding this comment

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

LGTM. @wasabeef can you help to merge this PR?

@thangnc thangnc mentioned this pull request Sep 10, 2022
Closed
@wasabeef
Copy link
Member

@Cu-Toof @thangnc
Thank you! I just merged this PR.

@thangnc
If you can continue the code review for FlutterGen elsewhere PRs, I’m looking forward to continuing working with you.

@wasabeef wasabeef merged commit 95d9635 into FlutterGen:main Sep 12, 2022
@wasabeef wasabeef added this to the 4.4.0 milestone Sep 12, 2022
@wasabeef wasabeef mentioned this pull request Sep 12, 2022
Merged
@thangnc
Copy link

thangnc commented Sep 13, 2022

@wasabeef thks for trusting me. I will take a look on other PRs

@wasabeef wasabeef mentioned this pull request Sep 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thangnc thangnc thangnc approved these changes

@wasabeef wasabeef Awaiting requested review from wasabeef wasabeef is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.4.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Cu-Toof @wasabeef @thangnc