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

Remove multi-file DartPad usages from cheatsheet #5392

Merged
merged 10 commits into from
Dec 22, 2023
Merged

Remove multi-file DartPad usages from cheatsheet #5392

merged 10 commits into from
Dec 22, 2023

Conversation

MaryaBelanger
Copy link
Contributor

@MaryaBelanger MaryaBelanger commented Dec 5, 2023

Part of #5382

  • The tests feature contained main for each example, but that features being removed. The examples need main to run, and also are not helpful without the feedback that the tests provided, so we're adding the tests to code visible in each dartpad.

    • Comment at the top of each that this part is tests, don't edit
    • Will have to adjust the tests for more useful messages to users.
    • Since main is a big focus now, add a new section on the main function at the top of the cheatsheet.
  • Also remove hint and solution feature and place in expandable "solution" section under each example.

  • Rewrite the intro section to explain the new way the page works.

Copy link

github-actions bot commented Dec 5, 2023

Visit the preview URL for this PR (updated for commit 449e0c5):

https://dart-dev--pr5392-cheatsheet-crrjrj27.web.app

(expires Fri, 29 Dec 2023 00:14:03 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: d851bc446d3c4d7394c5406c6f07255afc7075f3

@parlough
Copy link
Member

parlough commented Dec 5, 2023

Thanks for raising your concerns. I think we can still figure out something useful here.

Do you have time to meet for 30 minutes today (12/5)? Might be easier to discuss our options synchronously.

@MaryaBelanger
Copy link
Contributor Author

@parlough I was already planning to leave the new section on main for another PR, just to get this one finished. But then I saw in the intro that it says:

The Dart language is designed to be easy to learn for coders coming from other languages, but it has a few unique features. This codelab walks you through the most important of these language features.

In that light, going over main doesn't really fit to the theme of the page. What do you think?

@MaryaBelanger MaryaBelanger marked this pull request as ready for review December 18, 2023 22:39
@parlough
Copy link
Member

parlough commented Dec 18, 2023

I think it's fine to move to a follow-up CL, but I'd say it does fit the theme of being a unique (and cool) feature to Dart. Python and JS don't have a main method, Java (until a few weeks ago) required a complicated static public void main(String[] args) in a class, and C# requires something similar.

I'd also be happy to write it out if you think it's valuable. Just let me know!

@MaryaBelanger
Copy link
Contributor Author

MaryaBelanger commented Dec 18, 2023

Oh perfect, didn't know that and that info will help me write the section too. Created #5414 , thanks!

I'd also be happy to write it out if you think it's valuable. Just let me know!

If you'd prefer getting it into this PR, then by all means feel free to add it! But otherwise I'm happy to handle it too!

@parlough parlough self-assigned this Dec 19, 2023
Copy link
Member

@parlough parlough left a comment

Choose a reason for hiding this comment

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

This is great @MaryaBelanger!! Thanks so much for taking this on, especially unplanned and on a short notice.

I was worried how it would turn out without all of the original DartPad functionality, but it's not too different and I think the core benefits of the page still remain.

I left a few suggestions and questions, but nothing blocking.

A few sample tests do need still need to be updated to move away from _result, but I'd be happy to do that (and any of the other comments) as follow-up if you have other things you have to/prefer to work on right now. Just let me know :)

src/codelabs/dart-cheatsheet.md Outdated Show resolved Hide resolved
src/codelabs/dart-cheatsheet.md Outdated Show resolved Hide resolved
src/codelabs/dart-cheatsheet.md Outdated Show resolved Hide resolved
src/codelabs/dart-cheatsheet.md Outdated Show resolved Hide resolved
src/codelabs/dart-cheatsheet.md Outdated Show resolved Hide resolved
src/codelabs/dart-cheatsheet.md Outdated Show resolved Hide resolved
src/codelabs/dart-cheatsheet.md Outdated Show resolved Hide resolved
src/codelabs/dart-cheatsheet.md Outdated Show resolved Hide resolved
src/codelabs/dart-cheatsheet.md Outdated Show resolved Hide resolved
src/codelabs/dart-cheatsheet.md Outdated Show resolved Hide resolved
@parlough parlough changed the title Remove dartpad from cheatsheet Remove multi-file DartPad usages from cheatsheet Dec 20, 2023
@parlough parlough assigned MaryaBelanger and unassigned parlough Dec 20, 2023
@MaryaBelanger
Copy link
Contributor Author

MaryaBelanger commented Dec 21, 2023

@parlough Thanks for the thorough review. I went through and there were a lot of _results still in use so I got rid of all of them. I also changed any "try" language in the solution/hint text to "use" like you suggested in a few places (there were a few more).

Edit: idk why the builds failing I updated docker in this PR... is that ok or should it be another PR?

Copy link
Member

@parlough parlough left a comment

Choose a reason for hiding this comment

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

Thank you so much, looks great!

@parlough parlough merged commit 0d27b4f into main Dec 22, 2023
9 checks passed
@parlough parlough deleted the cheatsheet branch December 22, 2023 10:34
atsansone pushed a commit to atsansone/site-www that referenced this pull request Jan 26, 2024
Part of dart-lang#5382 

- The tests feature contained `main` for each example, but that features
being removed. The examples need `main` to run, and also are not helpful
without the feedback that the tests provided, so we're adding the tests
to code visible in each dartpad.
  - Comment at the top of each that this part is tests, don't edit
  - Will have to adjust the tests for more useful messages to users. 
- Since `main` is a big focus now, add a new section on the `main`
function at the top of the cheatsheet.

- Also remove hint and solution feature and place in expandable
"solution" section under each example.

- Rewrite the intro section to explain the new way the page works.

---------

Co-authored-by: Parker Lougheed <[email protected]>
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.

2 participants