Closed tobiasKaminsky closed 8 years ago
@jancborchardt @davivel @rperezb @purigarcia
Yeah, I agree that the development process needs to be more transparent – especially when it seems opaque to you as the most active contributor @tobiasKaminsky. We need to fully use Github for every public issue (@rperezb @davivel etc use Jira internally), and use proper milestones and be more responsive.
There’s software like Huboard which acts as an interface to make Github more Jira-like if that’s needed – but honestly we also do fine in core without stuff like this.
cc @MTRichards @cmonteroluque @karlitschek I consider this very important.
Yes. I completely agree. We have to become better in all the mentioned areas. @cmonteroluque @rperezb
If I can help you, please let me know :)
thank you @tobiasKaminsky. The points are well taken. The issue with old PR's is not unique to android, to be sure, and we're trying to figure out a way to address that across the board. We can do better evaluating github PR's as part of our sprint planning (better yet as part of sprint grooming so it can be done without the pressure of building the individual sprint) and prioritize appropriately.
The team is working on 2-week sprints, so this is a very dynamic process. The last sprint, however, we had the equivalent of an internal hack week and the team had complete freedom for a week to work on projects of their own choosing. Thus, unless someone wanted to take the week and dedicated to open PRs (which might have been a worthy endeavor), they didn't have to. And I stand by the process as I wanted everyone in the team to really have complete say on their work. We're now back to the usual 2-week sprint schedule and shortly (the remainder of this sprint or certainly the upcoming one) we will emphasize more the focus on open PRs, in competition, as you can imagine, with other priorities that we also need to handle.
I hope this gives you more visibility. Your contributions are extremely welcome and your comments in this issue are the same. We're definitely trying to improve our visibility and use of github and other open mechanisms as a result. Thank you for reminding us that there's work to do, I agree with you.
@tobiasKaminsky thanks for opening this thread.
we do very appreciate your contributions to the Android app, indeed, hope to meet you personally in Berlin, will you go to the ownCloud conference?
Last week, we had the internal hack, so everyone worked on whatever they do like it such as: the widget that @masensio did. Besides, we have to take into account that we are on summer, so that not the whole team is at the office. Because of that we were not so active. On the other hand, as @cmonteroluque has described we organised our job in 2 weeks sprints, our plan is to reflect it on the github milestones, such as: https://github.com/owncloud/android/milestones/2015-sprint-17-current
When we include a pr in our process, not only the code is reviewed but also it´s QA checing it again different Android versions, tablets and handset as well as different ownCloud server versions and environments, this is why it´s taking us longer.
Having said that, yes, there are room for us to improve the process.
@MTRichards something for us to have a look next Wed :- )
@cmonteroluque @rperezb Thank you both for your detailed insights in the management and structure of your work. Now I also see why a PR is taking this long time in QA, if you are testing this much ;)
But the first point is remained open: It is not really transparent why old PRs are left out in sprint planning and instead very newly (and minor) issues are taken into. The main problem with these old PRs is that the code can be so much outdated that very large parts of it have to be rewritten. Maybe it would be a possibility to plan that every new PR should be merged into develop after 4 sprints / 8 weeks. Within this relatively short time the code changes should not be that big.
@rperezb I cannot participate to the ownCloud conference as I am getting married the day before :) I hope that I can join next year (if it is not the weekend 27-28.08.16) ;)
Since there is quite a number of old pull requests, I would suggest one whole sprint fully focused on the pull requests. This is needed to take care of the accumulated contributions. And after that, every regular sprint should contain one or more of the contributor pull requests.
Thoughts @cmonteroluque @MTRichards @rperezb
hey @tobiasKaminsky, congrats!
We have just created a new milestone, we have included those pr that are expected to be merged in the next weeks so that included in the next ownCloud Android version. Next sprint, it will be mainly community focus. We do very appreciate your contributions. Plus, help in the QA part is also appreciated. We are currently discussing how to make the QA process easier, any input is more than welcome!
Great :) If I can help you with QA (testing or planning/discussing tests) you can open a new issue and we can discuss it there.
I :+1: @tobiasKaminsky arguments and @jancborchardt take on it. Beeing a noobie on owncloud android I get Tobias, rewrote the styling to material isn't too big of a deal but required a lot of files to be touched which makes it harder over time to keep it up to date with the develop branch in case merge conflicts arise which gets more probable by the time a PR is not merged.
@tobiasKaminsky I want to repeat the "come to the conference" meme. There's travel support for you if money is a constraint - just shoot me an email.
@jospoortvliet if I remember correctly, Tobias gets married on the dates. ;D Anyone else though: @AndyScherzinger @Stoyicker @LukeOwncloud @Kernald if you want to come to the conf and hack on the Android app: There’s travel support, and you’re not required to be there all the days if it’s not possible – would just be awesome to meet in person and work together! :)
I would love to come but I am on vacation during that time unfortunately :( Else it would be really awesome to hack on the Android App and sit around the same table :)
Coming at the conference would be awesome. I'll definitely see if I can get some days off work!
Same for me.
@tobiasKaminsky @AndyScherzinger next time then. ;)
@Stoyicker @LukeOwncloud @Kernald if you can come, then register at https://owncloud.org/conf and contact Jos about the travel support. There will be a workshop about contributing to the Android app so you can join in there and either learn more or also help other prospective contributors. :)
Next year :) But it has to be another weekend ;) Not again the last weekend of august... ;)
I want to thank you for the redesign of the developing process. Especially the new labels (or using them) is very helpful.
@tobiasKaminsky Happy to hear. :-) Please always post here if you find something that can be improved. We take feedback very seriously!
As @MTRichards said here https://github.com/owncloud/android/pull/821#issuecomment-148368416, the devs are quite busy with other developments. Counting the open PR and (again) mention that it is getting harder to merge them the longer we wait, maybe it can be a good idea to say, that one open PR per week should be reviewed/tested. I can help you with bug fixing :)
I second @tobiasKaminsky's approach and could also help out with bug fixing. Having a community PR merged per week would make life a lot easier since like Tobias mentioned maintaining this amount of PRs I parallel get very painful over time until it is nearly impossible to merge at all.
But like Tobias said I am also aware that this is simply a capacity issue.
One community PR merged per week is maybe a little bit to eager and not something I would expect. But maybe that the main devs are working half a day each per week for the community PR, would be great. So that there is a more regular work and not a big rush and then two weeks "silence" (a little bit extreme though)
True, it probably also depends on this size of the PR, half of mine are quite big due to libraries.
@davivel @rperezb @masensio is there anything we could do from our side which would make it easier to get PRs merged, for example also filling out a template when creating a PR or some tasks we could or should already have done and documented that would make your lifes easier?
ey, yes we are trying to keep, at least, this average, 1 pr/week.
These days due to the commitment we are checking small pr or others helpful for the current features :relaxed:
My next week goal is to validate myself this one: https://github.com/owncloud/android/pull/1200 @tobiasKaminsky can you please add (y) if you agree with @AndyScherzinger implementation
thx!
BTW: I know, it is a small question, but what are "blue tickets"?
These are bugs that are related to support tickets.
Could some automated testing help speeding up things? I never used such tools but I imagine if contributors could use such a test which tests all activities, input fields, clicking, and swiping, etc. more problems could be found before QA starts. Would a contribution of such a test environment/tool be welcome? And could it improve speeding up integrating PRs?
AFAIR there is already a test suite for automated tests: https://github.com/owncloud/android/blob/master/automationTest/README.md
I think the most improvement would be, that many community member review and test PRs. Then the effort for the main devs will be much less.
I think the most improvement would be, that many community member review and test PRs. Then the effort for the main devs will be much less.
Yep, this is a very important point! When everyone just cross-reviews pull requests of others at https://github.com/owncloud/android/pulls, we can go through testing and finding issues much quicker. Just like we do in the server and other apps.
ccing community contributors @tobiasKaminsky @AndyScherzinger @LukeOwncloud @stoyicker @Kernald @Wikinaut @nickvergessen @LukasReschke @yulin2 @Spacefish @jab416171 @zmatsuo @MMMarcy please check out the pull requests of others and review them. :) https://github.com/owncloud/android/pulls
Thanks a lot to everyone of you!
Not sure what the best way to track bugs related to a pr is, currently, we are adding them to the same pr, not sure whether this is visible or it would be better to open a new ticket and add a reference to it on the pr description. What do you think?
@rperezb I think opening a new issue and link to the corresponding PR is the best idea. Then the PR can focus on code review and discussion about functionality.
great
At least on the server and in other repositories: When we open a pull request for which there is no issue yet, we do not bother to open a separate issue about it since it would kind of be duplication. If the implementation of that PR should not go through (which is rarely the case without any follow-up), we can still open a more general issue.
@rperezb The way you are dealing with bugs in #1168 is great and we should keep this way: If we find a bug reference it in the first post of the corresponding PR! :+1:
great to know your feedback!
Continuos improvement!
@rperezb We have now 51 open PRs and they are getting even more... The idea "1 (even small) community PR per week" seems not to work :/ I thought that the beta programm will speed up the whole process. At least this is the reason I am doing this. But there are lots of PRs that are long time in beta and should be fine as they are tested by many users. And these PRs still not get reviewed by you, @davivel, @masensio or being validated by CA. So I think I can abandon the beta program as it is not helpful...?
Or asking the other way: where is the cause that the PRs are not dealt with? Lack of manpower? I have seen that in iOS someone wrote a test case for his own PR. Maybe it is in option that the community help with the QA and write also test cases/plans? With this way the "main" devs only have to accept the plans (and maybe test it only once on their own).
It is very very hard to maintain this many PRs. And it is getting even harder they older they are as the code changes quite a lot.
So, for me I will not do further PRs for the moment... (But I will try to fix the bugs already found in beta...)
IMO one of the problem is that there is only single person which is saying which PR can enter the codebase and which doesn't. This also discourages new people from pushing new PR, eg on android-library there was a very simple PR which was reviewed after 10months! and closed because of lack of response from original author, this is unacceptable. I understand that @davivel and @masensio are mostly working on stuff with closed source but android app should be also developed by community. Even recent blog post on owncloud said that android app is a example of such approach, when this is not true. There should be more people allowed to merge PR into source code. Since @tobiasKaminsky is heavily involved into app I would suggest that he should be allowed to do so (and probably even paid for this). So I call to @MTRichards and @karlitschek for some decisions.
Also @tobiasKaminsky I really admire your will to push new great PR's even when your work is omitted.
Maybe the push strategy of commiting changes (PRs) to the official repo and then waiting for them to be accepted, isn't the most appropriate approach. Instead there could be an unofficial repo - truly maintained and driven by the community. All contributions go there (first). When it is decided that some feature is to be included in the official repo, the corresponding source lines can be pulled (pulling whole PRs will probably not be possible anyways as the code bases will diverge over time).
This way we still have to versions (like now), but maintenance will be alleviated greatly.
Having two repos will introduce even bigger pull queue and merging problems
Hmmm, sensitive topic....
I totally get @tobiasKaminsky's frustration and I have also been there a couple of weeks ago and @tobiasKaminsky and others motivated me to not take a break. It happened for the same reason: having several PRs to maintain which where done months/weeks ago but keeping them on par with the master branch ended up in having merge conflicts every couple of weeks which is frustrating since the PR is done, it works and just the "not getting it merged" thing generates constant extra work that doesn't provide any extra value.
I don't want to and simply also cannot criticized anyone since I have no insight into the "main" devs work schedule, priorities and so on so I don't know what is the cause of the PRs not being taken care of since I am sure that they add a lot of value to the product.
To me it seems that the issue here is the lack of resources as in time. So I would ask the same questions @tobiasKaminsky has already raised in his latest comment: What can be done or needs to be done to speed up the merging of PRs. What are the criteria that need to be met to allow the merge of a PR (looking at most of the open PRs at least @tobiasKaminsky, @przybylski and mine there are DONE development wise, they got their fair share of bugfixes and they are all running out in the wild via the beta release for weeks/months, so I consider most of them very stable and mature). Or is it a matter of priorities/product strategy which would mean the community needs more information about the direction to which ownCloud wants the app to move to and we basically developed features that aren't in the focus of product management (I doubt that since most of the PRs are either enhancements or missing features compared to the other oC client implementations).
The beta concept worked very well to get feedback, find bugs and get fixes out to the users very fast, but the problem stays the same. The beta worked well and would probably work well in the future but as long as PRs don't get merged back the efforts to keep the beta in sync grows and grows since the number of PRs to be included/merged keeps growing. This problem would also be there even if we open up a separate repo which is basically the same thing that we have with the beta branch already.
So I am looking forward to your input @MTRichards, @karlitschek, @rperezb, @davivel and @masensio if there is anything that needs to be changed or anything the community can to to help speeding up things :)
@tobiasKaminsky WOW! Just checked and saw that you have 19 open PRs!!! Kudos to you that you have been able to maintain that many separate branches and yes this is definitely impossible to keep in sync.
In general how it the process for PRs and their merge in the server project being handled?
@tobiasKaminsky @AndyScherzinger @LukeOwncloud @przybylski @Kernald @stoyicker thanks a lot for your comments, and sorry. There’s really no excuse for this and we need to make sure to vastly improve this situation.
@tobiasKaminsky extra big sorry to you because you’ve been involved for so long, do so much awesome work, and yet wait for so many pull requests to be reviewed. :\
(I personally have to say I really like the idea of giving community people more review power, like doing QA, thumbs-up and probably merging as well.)
To be completely transparent, we are looking at options to balance workloads better and put more time into exactly what you requested - and to give more review power. Thank you (all of you) for the smack upside the head.
Working on it! We are spread across quite a few time zones, but we want to fix this quickly.
But there are lots of PRs that are long time in beta and should be fine as they are tested by many users.
That reasoning is false. Many testers doesn't grant that the object of test is fine. First, the target of tests in software development is, by definition, finding problems, not granting correction. Second, the fact that a feature works fine doesn't mean that it's an appropiate feature for the product.
Second, the fact that a feature works fine doesn't mean that it's an appropiate feature for the product.
Most of that is already made sure by @tobiasKaminsky @AndyScherzinger and everyone working together with me to get any new stuff gauged for viability and design reviewed.
Many testers doesn't grant that the object of test is fine.
Of course not, but every process has its holes and errors do slip through (like the current battery life issue). Up to a certain point (which we definitely are not yet at), more dedicated testers usually mean more issues found quicker.
Hello everyone,are there any Chinese here? Or who can speak Chinese?
@jiaojohn This open source project is using english as standard language.
@karlitschek Crying in the corner ,I hava a poor english
But there are lots of PRs that are long time in beta and should be fine as they are tested by many users.
That reasoning is false. Many testers doesn't grant that the object of test is fine. First, the target of tests in software development is, by definition, finding problems, not granting correction. Second, the fact that a feature works fine doesn't mean that it's an appropiate feature for the product.
You are of course right. I do not wanted to say that a PR that is long in beta need not to be tested by QA. But obvious failure or misleading UI or wrong UX can be tested with this. And of course some bugs are also being revealed and this makes the progress of QA faster as they find fewer bugs.
Follow up on our side, the next step here starting next Thurs, we will put a full time person on these requests. We will see how that goes, and also discuss other options - just letting you know one we can do immediately. We need to get through this backlog. Thank you all for your patience.
I am participating since a quite long time to the android app. But still I have some questions about the develop process. This should not be a rant, I am just curios and want to understand the internal processes better. I do not want to affront somebody. (And of course I am not quitting the development ;) )
Please try to answer my questions :)