First Pull Request Help Regarding Testing

Hi guys, I made this draft PR Media Sorting Improvement by Leo-Berman · Pull Request #16425 · nextcloud/android · GitHub, and I was looking at creating tests, but it seems like we would need to extract the logic and add a test with duplicate logic. This seemed like it would be deprecated soon and not be super useful. I’m not (at all) a kotlin developer and I just wanted to make this change because the way it previously worked wasn’t my favorite

Hello and welcome to the forum @Leo-Berman.

The change you are suggesting will change the default behavior of the app. This is typically not so happily accepted in general (not only NC) as is conflicts with user expectations (who are used to a certain behavior). Obvious errors/bugs are an exception of course.

I am no Kotlin dev as well, so I can only give you general statements. Eventually, you can seek advice as part of the PR chat. This is where this discussion mostly belongs. Also, the repo maintainers might have dedicated requirements for acceptance and then they would chip in.

What do you mean by duplicating logic in this context? Your PR does not contain tests so far.

1 Like

Hi Christian, thanks for your feedback.

Do you think I can add a settings for this, or should I abandon the PR entirely? Makes more sense to me to go by the folder structure if it’s there since that is the subfolder option available with the autoupload.

This isn’t something available to everyone correct?

I don’t really write kotlin and it seemed like testing would mean rewriting it somewhere else but I’ll take a deeper look into how to expose the logic if you think this PR is worth pursuing.

I can only stress this one: get in touch with the maintainer(s) and get your answers from them. If this was about the cookbook (where I am the maintainer of), I would expect so in case of doubt. Your PR is a draft, so any maintainer might think you are still busy and not assist you with your suggested changes. So, I once more encourage you to ask the maintainer of the app.

Hi Christian, I don’t really know the process behind open source contributions. Is there a proper channel to go about having a conversation with maintainers? Feels like flooding their inboxes wouldn’t be the best way to go about it. I set my PR to not draft, is waiting for a review the standard way to go about it?

Once again, very appreciative of your time.

Best,
Leo

In your pull request, you have the option to chat. If you scroll all the way down, there is a text box (here I am logged in, so my avatar is there):


The maintainers will see that (and probably get notified at least if you mention them. Then, they can have a look, give feedback and steer your work in the right direction.