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

Fixed double quotes, added override annotation #2592

Merged
merged 4 commits into from
Aug 28, 2020

Conversation

iLoveDocs
Copy link
Contributor

  • Replaced double string quote with single.
  • Added override annotation to make it easier to read that we're overriding a method from parent, because the heading itself talks about do this when overriding a method, (not sure if this should be done in docs but as guidelines suggest we should do that)
  • Added . at the end of comment (according to Dart comment style).

These are not the major changes in any way, but it is my first PR, so not sure how to do a pull request, I'll surely be able to make some significant changes in other pages of Dart once I realise how things work.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). 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 (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no Contributor has not signed the Contributor License Agreement label Aug 22, 2020
@iLoveDocs
Copy link
Contributor Author

@googlebot I signed it

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@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 Aug 22, 2020
@kwalrath
Copy link
Contributor

kwalrath commented Aug 24, 2020

@munificent do these changes LGTY? I'll figure out what happened to the build.

Oops, I assumed these changes were to the null safety article, but they're to older docs.

@kwalrath
Copy link
Contributor

Ah, the build break is just because the code was updated in the .md files but not the .dart files they auto-include from. E.g. the first snippet you changed was auto-included using the following, which means that the build will fail until we update examples/strong/lib/strong_analysis.dart:

<?code-excerpt "strong/lib/strong_analysis.dart (opening-example)" replace="/list(?=\))/[!$&!]/g"?>

Do you have (or want to learn) the git knowledge to update this PR with the relevant Dart files, or would you like me to add to this PR?

@kwalrath kwalrath requested review from domesticmouse and removed request for munificent August 24, 2020 22:57
@kwalrath
Copy link
Contributor

@domesticmouse can you take a look at the code?

@domesticmouse
Copy link
Member

Code changes LGTM. I'll hold the proper approval until the changes are migrated to the relevent Dart files =)

@iLoveDocs
Copy link
Contributor Author

@kwalrath

Oops, I assumed these changes were to the null safety article, but they're to older docs.

I actually wanted to make the changes in the null safety page but that page was too long for my first edit. So, I decided to edit the current page.

Ah, the build break is just because the code was updated in the .md files but not the .dart files they auto-include from.

I forgot to make the changes in the .dart file.

Do you have (or want to learn) the git knowledge to update this PR with the relevant Dart files, or would you like me to add to this PR?

I have no knowledge of it, but would love to learn that. I have a confusion though. Let's see there are 5 code changes made by me in the .md file and those 5 code snippets belong to 5 different .dart files in the example folder, I am supposed to edit each of those files, right? And if yes, then how do I upload those files after editing to this PR page? Isn't it too much of work; editing files in 2 places with exact same changes? Maybe I am not doing it the right way.

@kwalrath
Copy link
Contributor

If you have a local copy of the git repo, and you have the build working, then you only update the code in the .dart files, and then run a tool to update the .md file. But getting the build to work locally takes time, and some people never get it to work due to local configuration issues.

I'll be happy to make the changes locally and upload them to this PR.

@iLoveDocs
Copy link
Contributor Author

If you have a local copy of the git repo, and you have the build working, then you only update the code in the .dart files, and then run a tool to update the .md file. But getting the build to work locally takes time, and some people never get it to work due to local configuration issues.

I don't have a local copy, and I am sure I'd be one of those.

I'll be happy to make the changes locally and upload them to this PR.

How nice of you!

And, does it mean that the next time I make some changes in any of the Dart documentation by editing .md file only, I'll eligible to create a corresponding PR and you guys can do the local build on your own? (I mean is it OK to leave that part for you people or am I supposed to update the .dart files anyhow?)

@kwalrath
Copy link
Contributor

And, does it mean that the next time I make some changes in any of the Dart documentation by editing .md file only, I'll eligible to create a corresponding PR and you guys can do the local build on your own? (I mean is it OK to leave that part for you people or am I supposed to update the .dart files anyhow?)

Yes, it's fine, at least for now. Getting good fixes in trumps inconvenience right now. I'm hoping that, over time, we'll make it easier to update code and/or we'll get help from more people who are comfortable building the site.

@kwalrath kwalrath self-assigned this Aug 25, 2020
@kwalrath
Copy link
Contributor

I'm starting on this now.

@kwalrath
Copy link
Contributor

kwalrath commented Aug 28, 2020

Woot! Tests are passing! (The beta build broke, but that's allowed, and I'll try fixing that separately.)

@kwalrath kwalrath merged commit 6368ca1 into dart-lang:master Aug 28, 2020
@iLoveDocs
Copy link
Contributor Author

iLoveDocs commented Sep 3, 2020 via email

@kwalrath
Copy link
Contributor

kwalrath commented Sep 8, 2020

@iLoveDocs We should add a comment about the first solution not working under null safety. (See #2621 (comment) for a proposal.)

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.

4 participants