update documentation line indicating all List items must be same size

[hkparker]

Description:

Remove the line in the documentation that indicates that widget.List can only contain items of the same size. This is no longer true since the introduction of SetItemHeight in 2.3

Addresses the confusion that led to #4777.

Checklist:

• Tests included.
• Lint and formatter run with no errors.
• Tests all pass.

[andydotxyz]

I would not remove it, though I agree it is no longer correct.
How about indicating that /by default/ all items are the same size, but that SetItemHeight can be used to change this for a specific row?

[hkparker]

yeah good idea, let me know how this language looks

[Jacalz]

Please don't force-push. It usually breaks GitHub's ability to track comments from previous reviews., We'll squash on merge if we think it is needed. Also please fill out the checklist in the PR template as much as possible. Makes things a bit nicer to review for us.

Also FYI: It is a good idea to press the button to re-request a review after making changes :)

[hkparker]

Ach, sorry, I remembered squashing existed right after I did that. It's been a while. Let me know if you want me to open a PR without that. I haven't checked the checklist because it hasn't happened yet (I didn't include tests for this of course, an the pipeline hasn't run yet so technically I don't know if the linters and tests passed). But if I should just check everything since it's a comment change will do. And thanks for the FYI on re-review :)

[Jacalz]

Ach, sorry, I remembered squashing existed right after I did that. It's been a while. Let me know if you want me to open a PR without that.

Nah, it's fine. You had no open inline comments so nothing broke really.

I haven't checked the checklist because it hasn't happened yet (I didn't include tests for this of course, an the pipeline hasn't run yet so technically I don't know if the linters and tests passed).

Your PR is obviously just updating a comment so this is more a theoretical problem than anything. However, a good practice is to run the tests and linting locally to test things out before opening a PR (at least if you are modifying code). The idea is to fill out the checkboxes before opening the PR. Tests and linting ought to still complete in your case ;)

And thanks for the FYI on re-review :)

No problem. We've all been there. Happy to help out :)

[hkparker]

Ah yeah those comments, ok cool well I'll avoid a force push in the future.

Got it, can run all the tests locally for any code change, thanks for the context. Updated the PR description with checked boxes.

[Jacalz]

Updated the PR description with checked boxes.

Now I'm really picky. Sorry about that, but you haven't added any tests so that checkbox should of course not be checked.

[hkparker]

Of course! Updated the description

[Jacalz]

Thanks. Just one minor grammatical suggestion :)

[coveralls]

coverage: 64.898% (-0.008%) from 64.906%
when pulling 5c7e0a8 on hkparker:patch-2
into 213440e on fyne-io:develop.

[Jacalz]

Thanks. I can't merge until Andrew has re-reviewed but it looks great now. Thanks for the contribution and sorry for all a the back and forth :)

Requested changes were made

[dweymouth]

I dismissed Andy's review and went ahead and merged this since it's so straightforward

[Jacalz]

I dismissed Andy's review and went ahead and merged this since it's so straightforward

Cool. Didn't know that was a thing 🙃

[hkparker]

@Jacalz Ah the back and forth is appreciated, good to learn what I should do next time!

[Jacalz]

Cool. Thanks for you contribution :)