Migrate markdown to null safety and to packages repo #74757

Activity

1. Hixie

   added

   c: contributor-productivityTeam-specific productivity, code health, technical debt.Team-specific productivity, code health, technical debt.

   packageflutter/packages repository. See also p: labels.flutter/packages repository. See also p: labels.

   team-ecosystemOwned by Ecosystem teamOwned by Ecosystem team

   a: null-safetySupport for Dart's null safety featureSupport for Dart's null safety feature

   P3Issues that are less important to the Flutter projectIssues that are less important to the Flutter project

   c: tech-debtTechnical debt, code quality, testing, etc.Technical debt, code quality, testing, etc.

   on Jan 26, 2021
2. domesticmouse

   self-assigned this

   on Jan 26, 2021
3. domesticmouse commented on Jan 26, 2021

   

   domesticmouse

   on Jan 26, 2021

   Contributor

   More actions

   /cc @mjordan56 who is interested in migrating flutter_markdown to null safety.
4. domesticmouse commented on Jan 29, 2021

   

   domesticmouse

   on Jan 29, 2021

   Contributor

   More actions

   I'm curious what is required to port flutter_markdown to the packages repo?
5. Hixie commented on Jan 29, 2021

   

   Hixie

   on Jan 29, 2021

   ContributorAuthor

   More actions

   Nothing special, just making sure it fits the pattern in the other repo (changelog, readme, license, that kind of thing).
6. thisisamank commented on Jan 29, 2021

   

   thisisamank

   on Jan 29, 2021

   More actions

   @domesticmouse I am interested in helping, but I am a newbie to open source so I will need some guidance.
7. domesticmouse commented on Jan 29, 2021

   

   domesticmouse

   on Jan 29, 2021

   Contributor

   More actions

   Hey @thisisamank,

   You can see my initial attempt at migrating to null safety, but it's tests are currently reporting red. Feel free to pull my PR and see if you can figure out what changes are required to make Mockito happy.

   Thanks!
8. mjordan56 commented on Jan 29, 2021

   

   mjordan56

   on Jan 29, 2021 via email

   More actions

   Good morning Brett, I've been looking at PR #324, your null safety first attempt. I figured out what was failing in the link_test.dart tests. It's interesting to see how some of the code reacts to the null safety changes. I assume you did the "dart migrate" command to get these changes. I noticed at least one instance of a null safety change being redundant. I won't hurt anything but it wasn't necessary either. I'm looking at some of the other tests that are failing. I just started looking at the image_test.dart tests. At this point, this seems to be a MockHttpClient problem. So, how do you want to coordinate our effort? I don't want to waste my time if you or others are already working on something. Also, how do you want to take fixes? I created a local branch in my fork with your PR. Do you want us to merge our changes into your PR? I haven't done this before but GitHub says this is doable. https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/committing-changes-to-a-pull-request-branch-created-from-a-fork https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork It looks like you have to grant permission to do this, which makes sense. I have a change for the link tests now if you want them. Michael

   …

   On Fri, Jan 29, 2021 at 2:25 PM Brett Morgan ***@***.***> wrote: Hey @thisisamank <https://github.com/thisisamank>, You can see my initial attempt at migrating to null safety <https://github.com/flutter/flutter_markdown/pull/324>, but it's tests are currently reporting red. Feel free to pull my PR and see if you can figure out what changes are required to make Mockito happy. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#74757 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQECFT3HDMLXKCS2A7BPDDS4MRTRANCNFSM4WUEJC5Q> .
9. mjordan56 commented on Jan 29, 2021

   

   mjordan56

   on Jan 29, 2021 via email

   More actions

   Hi Brett, I've confirmed the image tests are failing due to a problem in utils/createMockImageHttpClient line 195. when(client.getUrl(any)) .thenAnswer((_) => Future<io.HttpClientRequest>.value(request)); The mock closure "any" has a return type of Null. I'm not sure of the correct solution to fix this problem. I can figure it out but I thought you or @thisisamank might have more insight into Mockito and know how to fix this bug. Anyhow, when I set the six image tests that fail to skip and with the fixes I have for the link tests, I'm able to successfully complete all of the tests with your PR. How would you like to proceed? Michael On Fri, Jan 29, 2021 at 3:03 PM Michael Jordan <[email protected]> wrote:

   …

   Good morning Brett, I've been looking at PR #324, your null safety first attempt. I figured out what was failing in the link_test.dart tests. It's interesting to see how some of the code reacts to the null safety changes. I assume you did the "dart migrate" command to get these changes. I noticed at least one instance of a null safety change being redundant. I won't hurt anything but it wasn't necessary either. I'm looking at some of the other tests that are failing. I just started looking at the image_test.dart tests. At this point, this seems to be a MockHttpClient problem. So, how do you want to coordinate our effort? I don't want to waste my time if you or others are already working on something. Also, how do you want to take fixes? I created a local branch in my fork with your PR. Do you want us to merge our changes into your PR? I haven't done this before but GitHub says this is doable. https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/committing-changes-to-a-pull-request-branch-created-from-a-fork https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork It looks like you have to grant permission to do this, which makes sense. I have a change for the link tests now if you want them. Michael On Fri, Jan 29, 2021 at 2:25 PM Brett Morgan ***@***.***> wrote: > Hey @thisisamank <https://github.com/thisisamank>, > > You can see my initial attempt at migrating to null safety > <https://github.com/flutter/flutter_markdown/pull/324>, but it's tests > are currently reporting red. Feel free to pull my PR and see if you can > figure out what changes are required to make Mockito happy. > > Thanks! > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#74757 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/ABQECFT3HDMLXKCS2A7BPDDS4MRTRANCNFSM4WUEJC5Q> > . >
10. Hixie commented on Jan 29, 2021

    

    Hixie

    on Jan 29, 2021

    ContributorAuthor

    More actions

    I believe that Mockito isn't supported in null-safety right now. The fix we've used elsewhere is migrating away from mockito.
11. 7 remaining items

    Load more
12. pedromassangocode

    added

    passed first triage

    on Feb 3, 2021
13. stuartmorgan-g commented on Apr 29, 2021

    

    stuartmorgan-g

    on Apr 29, 2021

    Contributor

    More actions

    @domesticmouse I'm doing some other migrations, and could do this one if you're ready. It looks like there are a few open PRs; any that you want to land before migration?
14. domesticmouse commented on Apr 29, 2021

    

    domesticmouse

    on Apr 29, 2021

    Contributor

    More actions

    Please go ahead and migrate. The outstanding PRs require work from the authors to bring them up to date, add tests, and the like.
15. stuartmorgan-g

    added a commit that references this issue on Apr 30, 2021

    [flutter_markdown] Import from flutter/flutter_markdown

    ...

    31947e6
16. stuartmorgan-g commented on May 3, 2021

    

    stuartmorgan-g

    on May 3, 2021

    Contributor

    More actions

    @domesticmouse Are you a member of the flutter.dev verified publisher on pub? If so, could you transfer https://pub.dev/packages/flutter_markdown over to the verified account (in the Admin tab)? If you're not, we should have @kf6gpe add you.
17. stuartmorgan-g commented on May 3, 2021

    

    stuartmorgan-g

    on May 3, 2021

    Last edited by stuartmorgan-g

    Contributor

    More actions

    • Merge landed
    • I closed the few remaining PRs with a message explaining what happened and what to do if they still want to move forward
    • I migrated all the issues to flutter/flutter and tagged them with p: flutter_markdown

    Once https://github.com/flutter/flutter_markdown/pull/368 is approved and landed we should be able to archive the repo.

    That plus getting https://pub.dev/packages/flutter_markdown transferred to the verified publisher so I can publish the updated meta should be all that's left here.
18. domesticmouse commented on May 4, 2021

    

    domesticmouse

    on May 4, 2021

    Contributor

    More actions

    @stuartmorgan I'm OoO this week. I don't know if I'm part of the blessed group, and I don't know the process to transfer ownership.

    @kf6gpe can I get some time on your calendar next week for this?
19. kf6gpe commented on May 4, 2021

    

    kf6gpe

    on May 4, 2021

    Contributor

    More actions

    @domesticmouse Sure thing! When you're back, toss something on my calendar --- it should only take us a few munutes.

    Enjoy (hopefully) your OoO-ness in the mean time.
20. stuartmorgan-g commented on May 4, 2021

    

    stuartmorgan-g

    on May 4, 2021

    Contributor

    More actions

    and I don't know the process to transfer ownership.

    Once you've been added to the verified publisher group, it's super-easy:

    • Go to https://pub.dev/packages/flutter_markdown/admin
    • Pick "flutter.dev" in the first dropdown
    • Press the red TRANSFER TO PUBLISHER button
21. stuartmorgan-g commented on May 12, 2021

    

    stuartmorgan-g

    on May 12, 2021

    Contributor

    More actions

    @domesticmouse Alternatively you could add me to the package (dart pub uploader --package=flutter_markdown add [email protected]) and I can transfer it.
22. domesticmouse commented on May 12, 2021

    

    domesticmouse

    on May 12, 2021

    Contributor

    More actions

    @stuartmorgan uploader invite sent. Check your inbox =)
23. stuartmorgan-g commented on May 13, 2021

    

    stuartmorgan-g

    on May 13, 2021

    Last edited by stuartmorgan-g

    Contributor

    More actions

    The publisher is converted over, and I published the update that has the new metadata (pointing to the new source location), so this is finished!
24. stuartmorgan-g

    closed this as completedon May 13, 2021
25. github-actions commented on Aug 1, 2021

    

    github-actionsbot

    on Aug 1, 2021

    More actions

    This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.
26. github-actions

    locked as resolved and limited conversation to collaborators on Aug 1, 2021