search

brailleapps / dotify.formatter.impl

Provides an implementation of the formatter interfaces in dotify.api
GNU Lesser General Public License v2.1
0 stars 6 forks source link

Code documentation in the form of Javadocs and comments #107

Closed bertfrees closed 5 years ago

bertfrees commented 5 years ago

This contains all the changes I did in https://github.com/brailleapps/dotify.formatter.impl/pull/102, with some improvements, and one or two changes from Paul are also included.

The improvements compared with https://github.com/brailleapps/dotify.formatter.impl/pull/102 are:

@kalaspuffar, @PaulRambags and @joeha480 What do you think? Is this OK to merge? We're still a long way from a full coverage, and things can be expanded and worded differently, but I think this is already a major improvement. We can further improve the documentation of specific parts of the code as we work on that code.

The remaining questions from Paul in https://github.com/brailleapps/dotify.formatter.impl/pull/102 (which is not much anymore) can be discussed and handled in a subsequent PR.

PaulRambags commented 5 years ago

I already approved this PR but please resolve the conflicts and rephrase the sentence.

bertfrees commented 5 years ago

I resolved the conflicts (rebased onto master).

However, just to be clear, I think it is not "automatically" the job of the creator of the PR to resolve conflicts with things that have been added to master afterwards. Of course if the person who does the merges (Daniel) asks for help I'm happy to give it a try. In this case it was easy because the commit on master was also from me.

kalaspuffar commented 5 years ago

Hi @bertfrees

In this case, where we have a local repository where all members have access to change and update the PR we could do it. But in most cases, the merge request has been created in the git repository of the submitting party. For instance, I have a fork where I do my changes before submitting, and changes to that fork only I have access to update.

And much of the work done here is in the sbsdev fork and maintaining those pull request needs to be done by the submitting party or other interested developers at sbsdev.

Best regards Daniel

bertfrees commented 5 years ago

Well I guess it's a matter of agreeing on something. I just think it's not so self-evident. Why don't we bring it up in the next meeting...

kalaspuffar commented 5 years ago

Hi @bertfrees

I've added the discussion point to the Meeting Agenda. Please verify the wording so we remember what needs discussing.

Best regards Daniel

bertfrees commented 5 years ago

Perfect, thanks.

bertfrees commented 5 years ago

@PaulRambags Don't worry, a new PR is coming soon.