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

Update hello world example to match current behavior #678

Merged
merged 4 commits into from Nov 15, 2017
Merged

Update hello world example to match current behavior #678

merged 4 commits into from Nov 15, 2017

Conversation

mattrenquist
Copy link
Contributor

The current hello world example does not run. Adding 'textDirection' to match the code in examples/hello_world/lib/main.dart fixes the bug and allows the code to run. The website documentation should be updated to match the current code.

Update hello world example with required textDirection as in examples/hello_world/lib/main.dart
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@googlebot googlebot added the cla: no Contributor has not signed the Contributor License Agreement label Sep 18, 2017
@mattrenquist
Copy link
Contributor Author

Signed the CLA on 10:30 PST 9/17/2017.

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes Contributor has signed the Contributor License Agreement and removed cla: no Contributor has not signed the Contributor License Agreement labels Sep 18, 2017
@Hixie
Copy link
Contributor

Hixie commented Sep 18, 2017

Can you add a space after the comma?

@Hixie
Copy link
Contributor

Hixie commented Sep 18, 2017

The Travis failure is because we're requiring the use of dartfmt, but here dartfmt is making the code uglier by wrapping in a poor way. We should probably exclude this file from dartfmt verification.

cc @sethladd

Added space after comma per request and formatting guidelines.
@mattrenquist
Copy link
Contributor Author

Added the space. Thanks.

@Hixie
Copy link
Contributor

Hixie commented Sep 18, 2017

or maybe we can tell dartfmt to accept longer lines? iirc @mit-mit solved our most recent issue with Travis in this repo, maybe he has a quick way to address this one too. :-)

@mit-mit
Copy link
Member

mit-mit commented Sep 19, 2017

dartfmt can be passed a --line-length argument to specify a longer wrapping length than it's default of 80. But we invoke dartfmt in a single location for all formating checks, so would need to make that change globally.

@mit-mit
Copy link
Member

mit-mit commented Oct 9, 2017

@Hixie @sethladd did you figure out how we want to handle this formatting issue?

@Hixie
Copy link
Contributor

Hixie commented Oct 13, 2017

I have no particular opinion on how to solve this. I don't think dartfmt can handle this code in a good way. I'd be fine with excluding this file, changing the line width globally (assuming it doesn't make anything else worse), not using dartfmt at all, or any other similar solution.

@Sfshaza
Copy link
Contributor

Sfshaza commented Oct 13, 2017

A longer-than-80-char line length means that code can wrap and look bad when displayed inside a web page.

@sethladd
Copy link
Contributor

Or, add trailing commas to the snippet, reformat it with dartfmt

@Hixie
Copy link
Contributor

Hixie commented Nov 15, 2017

@sethladd or @Sfshaza, can one of you update this to be wrapped as the Web site expects and then land this? Thanks!

@mit-mit
Copy link
Member

mit-mit commented Nov 15, 2017

I suggest we just add some trailing commas and auto-format, which would give us:

import 'package:flutter/material.dart';

void main() {
  runApp(
    new Center(
      child: new Text(
        'Hello, world!',
        textDirection: TextDirection.ltr,
      ),
    ),
  );
}

@sethladd
Copy link
Contributor

formatting lgtm

@mit-mit
Copy link
Member

mit-mit commented Nov 15, 2017

Pushed that change to this PR

@mit-mit
Copy link
Member

mit-mit commented Nov 15, 2017

Travis is happy now, merging.

@mattrenquist thanks for contributing, and sorry for being so slow with this PR.

@mit-mit mit-mit merged commit f5113a0 into flutter:master Nov 15, 2017
@mattrenquist
Copy link
Contributor Author

No worries. Glad I could help out even a little. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Contributor has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants