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

Feature - Haiku generator app #1461

Merged
merged 21 commits into from
Mar 14, 2023
Merged

Feature - Haiku generator app #1461

merged 21 commits into from
Mar 14, 2023

Conversation

st-nhanho
Copy link
Contributor

@st-nhanho st-nhanho commented Mar 7, 2023 •
edited
Loading

Haiku generator app

Pre-launch Checklist

  • I read the Effective Dart: Style recently, and have followed its advice.
  • I signed the CLA.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-devrel channel on Discord.

@st-nhanho st-nhanho changed the title Added codebase for haiku app Feature - Haiku generator app Mar 7, 2023
@domesticmouse
Copy link
Contributor

I'm curious what the context for this submission is? Does it support a codelab, and if so, which one?

domesticmouse
domesticmouse requested changes Mar 7, 2023
View reviewed changes
Copy link
Contributor

@domesticmouse domesticmouse left a comment

Choose a reason for hiding this comment

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

Without an attached codelab and a maintenance plan, this PR will not be landed.

st-tuanmai
st-tuanmai reviewed Mar 8, 2023
View reviewed changes
haiku_generator/lib/main.dart Outdated Show resolved Hide resolved
haiku_generator/lib/app.dart
),
),
)
: ShimmerLoadingAnim(
Copy link

Choose a reason for hiding this comment

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

loading shows after choice option (when data loading). please change to check from a data status

haiku_generator/lib/app.dart Outdated Show resolved Hide resolved
haiku_generator/lib/app.dart Outdated Show resolved Hide resolved
haiku_generator/lib/widget/shimmer_loading.dart Outdated Show resolved Hide resolved
haiku_generator/lib/widget/shimmer_loading.dart Outdated Show resolved Hide resolved
haiku_generator/lib/widget/shimmer_loading.dart Outdated Show resolved Hide resolved
@st-nhanho st-nhanho changed the title Feature - Haiku generator app [WIP] Feature - Haiku generator app Mar 8, 2023
domesticmouse
domesticmouse approved these changes Mar 8, 2023
View reviewed changes
Copy link
Contributor

@domesticmouse domesticmouse left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Happy to land once @windmaple is happy

@domesticmouse
Copy link
Contributor

PTAL @windmaple

@domesticmouse
Copy link
Contributor

You'll need to target this to codelabs:main instead of codelabs:master to make this PR landable

@domesticmouse
Copy link
Contributor

Also, please add this sample to https://github.com/flutter/codelabs/blob/main/flutter_ci_script_stable.sh and https://github.com/flutter/codelabs/blob/main/flutter_ci_script_beta.sh for CI

@st-nhanho st-nhanho changed the base branch from master to main March 8, 2023 03:47
@st-nhanho
Copy link
Contributor Author

You'll need to target this to codelabs:main instead of codelabs:master to make this PR landable

Yes, I have just change base branch

@st-nhanho
Copy link
Contributor Author

Also, please add this sample to https://github.com/flutter/codelabs/blob/main/flutter_ci_script_stable.sh and https://github.com/flutter/codelabs/blob/main/flutter_ci_script_beta.sh for CI

Yes, I will add this

@st-nhanho st-nhanho changed the title [WIP] Feature - Haiku generator app Feature - Haiku generator app Mar 8, 2023
@st-nhanho
Copy link
Contributor Author

@windmaple I have just fixed feedback
Could you review it?

@windmaple
Copy link
Contributor

@domesticmouse let's merge

@domesticmouse
Copy link
Contributor

@domesticmouse let's merge

Hey @windmaple, in theory, I believe you should be able to land it. Please wait until CI turns green.

If not, please send me an email to remind me to land it in the morning.

Thanks!

@domesticmouse domesticmouse merged commit 3419755 into flutter:main Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@windmaple windmaple windmaple left review comments

@st-tuanmai st-tuanmai st-tuanmai left review comments

@domesticmouse domesticmouse domesticmouse approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@st-nhanho @domesticmouse @windmaple @st-tuanmai