+1 for your proposed changes, Robert. I would argue that is even more crucial that big pull requests contain documentation because a lot of times only the contributor can create this documentation. Additionally, documentation makes reviewing a pull request much easier.
Fragmented documentation is a separate issue. We have to make sure that we check the quality of the documentation in addition to the quality of the submitted code. On Wed, Jun 3, 2015 at 11:57 PM, Fabian Hueske <fhue...@gmail.com> wrote: > +1 for Robert's suggestion. > > In fact, I thought this was already our practice. Also I would not allow > exceptions from that rule in the "stable" codebase. Writing documentation > and describing how stuff should be used lets you think about it in a > different way and can help to make the feature better. Also features which > are not documented do not exist. We might be less strict with changes to > flink-staging but as soon as it moves out, documentation must be > "complete". > > However, such a rule will only help to have (more) complete documentation, > but not necessarily good documentation. I observed that documentation tends > to cluttered and more fragmented if more people add and change stuff. I > guess from time to time, documentation just needs to be restructured and > partially rewritten. > > 2015-06-03 18:19 GMT+02:00 Vasiliki Kalavri <vasilikikala...@gmail.com>: > > > Hi, > > > > in principle I agree with Robert's suggestion. > > > > I can only see two cases where this might not work well: > > > > - if the change is something big / totally new and the documentation to > be > > written is large. In this case, I would prefer to have a separate issue > for > > docs to ensure better quality and review, otherwise we might end up with > > following "improve docs" issues > > > > - if the code change is blocking some other issue. Then, it might make > > sense to merge the code fast and update the docs fast. > > > > In any case though, I agree that it's not a good idea to have someone > > adding the missing docs just before the release. > > > > Cheers, > > -Vasia. > > > > On 3 June 2015 at 17:27, Till Rohrmann <trohrm...@apache.org> wrote: > > > > > For me it also makes sense that the contributor who has implemented the > > > changes and thus knows them best should update the documentation > > > accordingly. > > > > > > +1 > > > > > > On Wed, Jun 3, 2015 at 5:25 PM, Chiwan Park <chiwanp...@icloud.com> > > wrote: > > > > > > > +1 Good. PR should contain documentation. > > > > > > > > Regards, > > > > Chiwan Park > > > > > > > > > On Jun 4, 2015, at 12:24 AM, Lokesh Rajaram < > > rajaram.lok...@gmail.com> > > > > wrote: > > > > > > > > > > +1. I like this idea, not sure if my vote counts :) > > > > > > > > > > On Wed, Jun 3, 2015 at 8:21 AM, Robert Metzger < > rmetz...@apache.org> > > > > wrote: > > > > > > > > > >> Hi, > > > > >> > > > > >> as part of making our codebase ready for the upcoming 0.9 release, > > > I've > > > > >> started to go over the documentation of Flink. > > > > >> > > > > >> It seems that our current approach for documenting stuff: > > > > >> - We implement and merge a feature > > > > >> - We open a JIRA for documenting it. > > > > >> > > > > >> Before the release, we realize that we have many open > documentation > > > > issues > > > > >> (currently 26) and hope that somebody (in this case me) is fixing > > > them. > > > > >> > > > > >> Some of the pull requests also contain documentation, but > certainly > > > not > > > > all > > > > >> of them. > > > > >> > > > > >> > > > > >> I am proposing to: > > > > >> - add a rule to our coding guidelines [1] which states that every > > > change > > > > >> that affects the documentation needs to update the documentation > > > > >> accordingly. > > > > >> - Committers have to make sure that pull request are following the > > > rule > > > > >> before accepting the change. Otherwise, we reject the pull > request. > > > > >> > > > > >> > > > > >> [1] http://flink.apache.org/coding-guidelines.html > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
