[jira] [Commented] (OFBIZ-9877) [Refactoring] Package org.apache.ofbiz.accounting.tax

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (OFBIZ-9877) [Refactoring] Package org.apache.ofbiz.accounting.tax

Nicolas Malin (Jira)

    [ https://issues.apache.org/jira/browse/OFBIZ-9877?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16293765#comment-16293765 ]

Jacques Le Roux commented on OFBIZ-9877:
----------------------------------------

Michael,

bq. I fully understand your concerns and we don't want to introduce too much formatting changes. From what I understand, the team has used a formatting style based on a formatter file for Eclipse, like I do. I cannot find the download link for it right now, but the understanding was that this is the recommended style. I have it in Eclipse for years.
Actually there has never been a consensual agreement on that with the team. It's me that years ago proposed my own formatter, after few discussions I can't recall nor want to find. I recently had a look at it. It's there as an attachement of https://cwiki.apache.org/confluence/display/OFBIZ/Coding+Conventions but for a reason no longer downloadable. So I added the one I use today:
https://cwiki.apache.org/confluence/pages/viewpageattachments.action?pageId=7766027

bq. In [1] is stated that we use the Sun coding standards, which seems not to be the case. For example, they recommend a line width of 80 characters (which is wayy too short for my taste). maybe we should get rid of this part in the wiki or do some updates.
+1

bq. I assume that there is no real coding convention used by EVERY developer or at least by EVERY committer so you will always have different formatting styles. You can really see the different styles in the code base. Some people do not seem to use formatting at all.
Right

bq. I would really like to have a coding style where we can rely on, that would make things much easier to maintain an even looking code. Maybe we should decide on a mandatory formatting and do a big reformatting session with no functional changes to have a common ground to continue with. But this is another issue.
Yes, not a top priority and not the subject of the current discussion

bq. Formating vs. refactoring: we have provided the contents of this series of patches for refactoring, see OFBIZ-9836, including some formatting changes (like {} around if/else statements). There should be no functional changes with the exception of multi-catches. The functional changes were in the FindBugs patches.
Here my point is refactoring is not formatting and because refactoring is by itself already something which needs careful reviews, adding lines of formatting, mix 2 types of things. So, the subject of the current discussion, it's not functional vs not functional; but no formatting, especially lines cut, in refactoring commits.

bq. So before I do the commit work for the OFBIZ-9836 patches (which was planned for today, better say: NOW), we have to decide how to proceed. There is much work done and a i have not the capacity to change all the formatting which might be done in the patches.
OK let it be for this patches I'll try to review and will refrain myself for reformatting things which I dislike (yes it's often a feeling rather than something you can put a rule on, though the EntityQuery.use(delegator).from() stuff I guess we all agree it's easier to read when splitted, except very short ones (yes there are always exception, such is Life, we can't get around)

Bq. Like the FindBugs, I would like to get this package of changes into the coming release branch. Our team wants to focus on functional enhancements in the next step and close this chapter shortly.
No problems let's go

bq. I see the following possible moves:

I agree on 1, let's go for now

bq. However we decide, we should take care of getting some binding coding conventions together to avoid confusion in the future.
Big +1, we need that really

Thanks to care.

> [Refactoring] Package org.apache.ofbiz.accounting.tax
> -----------------------------------------------------
>
>                 Key: OFBIZ-9877
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-9877
>             Project: OFBiz
>          Issue Type: Sub-task
>          Components: accounting
>    Affects Versions: Trunk
>            Reporter: Julian Leichert
>            Assignee: Michael Brohl
>            Priority: Minor
>             Fix For: Upcoming Release
>
>         Attachments: OFBIZ-9877_org.apache.ofbiz.accounting.tax.TaxAuthorityServices_refactoring.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)