Re: svn commit: r1818378 - /ofbiz/ofbiz-framework/trunk/a
Locked
 
|
I think it is always better to respect limit width because it has many
implications that affect other developers. Anyway, I do not think the right place to make arguments is in commit messages. A commit message is supposed to hold information on what was done, not a place to argue formatting conventions or what you'd like to do. Perhaps it is more appropriate to use the ML for that. On Dec 16, 2017 12:28 PM, <[hidden email]> wrote: Author: jleroux Date: Sat Dec 16 09:28:37 2017 New Revision: 1818378 URL: http://svn.apache.org/viewvc?rev=1818378&view=rev Log: No functional change, just trivial formatting I began to reformat this supposed refactoring commit, but stopped and decided to rather discuss this in the related Jira. BTW automatically cutting lines at some narrow width does not make sense most of the time. See EntityQuery cases for instance, are they not easier to read when, IMO, rightly formatted, as I did here? Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/app lications/accounting/src/main/java/org/apache/ofbiz/accounti ng/tax/TaxAuthorityServices.java?rev=1818378&r1=1818377&r2=1818378&view=diff ============================================================ ================== --- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java (original) +++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java Sat Dec 16 09:28:37 2017 @@ -63,8 +63,7 @@ public class TaxAuthorityServices { public static final int salestaxRounding = UtilNumber.getBigDecimalRoundingMode("salestax.rounding"); public static final String resource = "AccountingUiLabels"; - public static Map<String, Object> rateProductTaxCalcForDisplay(DispatchContext dctx, - Map<String, ? extends Object> context) { + public static Map<String, Object> rateProductTaxCalcForDisplay(DispatchContext dctx, Map<String, ? extends Object> context) { Delegator delegator = dctx.getDelegator(); String productStoreId = (String) context.get("productStoreId"); String billToPartyId = (String) context.get("billToPartyId"); @@ -87,20 +86,27 @@ public class TaxAuthorityServices { } try { - GenericValue product = EntityQuery.use(delegator).fro m("Product").where("productId", productId).cache() + GenericValue product = EntityQuery.use(delegator) + .from("Product") + .where("productId", productId).cache() + .queryOne(); + GenericValue productStore = EntityQuery.use(delegator) + .from("ProductStore") + .where("productStoreId", productStoreId) + .cache() .queryOne(); - GenericValue productStore = EntityQuery.use(delegator).fro m("ProductStore").where("productStoreId", - productStoreId).cache().queryOne(); if (productStore == null) { - throw new IllegalArgumentException("Could not find ProductStore with ID [" + productStoreId - + "] for tax calculation"); + throw new IllegalArgumentException("Could not find ProductStore with ID [" + productStoreId + "] for tax calculation"); } if ("Y".equals(productStore.getString("showPricesWithVatTax"))) { Set<GenericValue> taxAuthoritySet = new HashSet<>(); if (productStore.get("vatTaxAuthPartyId") == null) { - List<GenericValue> taxAuthorityRawList = EntityQuery.use(delegator).from("TaxAuthority") - .where("taxAuthGeoId", productStore.get("vatTaxAuthGeoId")).cache().queryList(); + List<GenericValue> taxAuthorityRawList = EntityQuery.use(delegator) + .from("TaxAuthority") + .where("taxAuthGeoId", productStore.get("vatTaxAuthGeoId")) + .cache() + .queryList(); taxAuthoritySet.addAll(taxAuthorityRawList); } else { GenericValue taxAuthority = EntityQuery.use(delegator).from("TaxAuthority").where("taxAuthGeoId", @@ -175,7 +181,9 @@ public class TaxAuthorityServices { GenericValue facility = null; try { if (productStoreId != null) { - productStore = EntityQuery.use(delegator).fro m("ProductStore").where("productStoreId", productStoreId) + productStore = EntityQuery.use(delegator) + .from("ProductStore") + .where("productStoreId", productStoreId) .queryOne(); } if (facilityId != null) { |
Re: svn commit: r1818378 - /ofbiz/ofbiz-framework/trunk/a
Administrator
|
Actually we had a good discussion in the related Jira https://issues.apache.org/jira/browse/OFBIZ-9877
We agreed it's time to rediscuss this again and I agree/and-suggested-there the dev ML is the best place for that Jacques Le 16/12/2017 à 14:27, Taher Alkhateeb a écrit : > I think it is always better to respect limit width because it has many > implications that affect other developers. > > Anyway, I do not think the right place to make arguments is in commit > messages. A commit message is supposed to hold information on what was > done, not a place to argue formatting conventions or what you'd like to do. > Perhaps it is more appropriate to use the ML for that. > > On Dec 16, 2017 12:28 PM, <[hidden email]> wrote: > > Author: jleroux > Date: Sat Dec 16 09:28:37 2017 > New Revision: 1818378 > > URL: http://svn.apache.org/viewvc?rev=1818378&view=rev > Log: > No functional change, just trivial formatting > > I began to reformat this supposed refactoring commit, but stopped and > decided > to rather discuss this in the related Jira. > > BTW automatically cutting lines at some narrow width does not make sense > most of > the time. > See EntityQuery cases for instance, are they not easier to read when, IMO, > rightly formatted, as I did here? > > Modified: > ofbiz/ofbiz-framework/trunk/applications/accounting/src/main > /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java > > Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main > /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java > URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/app > lications/accounting/src/main/java/org/apache/ofbiz/accounti > ng/tax/TaxAuthorityServices.java?rev=1818378&r1=1818377&r2=1818378&view=diff > ============================================================ > ================== > --- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main > /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java (original) > +++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main > /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java Sat Dec 16 > 09:28:37 2017 > @@ -63,8 +63,7 @@ public class TaxAuthorityServices { > public static final int salestaxRounding = > UtilNumber.getBigDecimalRoundingMode("salestax.rounding"); > public static final String resource = "AccountingUiLabels"; > > - public static Map<String, Object> > rateProductTaxCalcForDisplay(DispatchContext > dctx, > - Map<String, ? extends Object> context) { > + public static Map<String, Object> > rateProductTaxCalcForDisplay(DispatchContext > dctx, Map<String, ? extends Object> context) { > Delegator delegator = dctx.getDelegator(); > String productStoreId = (String) context.get("productStoreId"); > String billToPartyId = (String) context.get("billToPartyId"); > @@ -87,20 +86,27 @@ public class TaxAuthorityServices { > } > > try { > - GenericValue product = EntityQuery.use(delegator).fro > m("Product").where("productId", productId).cache() > + GenericValue product = EntityQuery.use(delegator) > + .from("Product") > + .where("productId", productId).cache() > + .queryOne(); > + GenericValue productStore = EntityQuery.use(delegator) > + .from("ProductStore") > + .where("productStoreId", productStoreId) > + .cache() > .queryOne(); > - GenericValue productStore = EntityQuery.use(delegator).fro > m("ProductStore").where("productStoreId", > - productStoreId).cache().queryOne(); > if (productStore == null) { > - throw new IllegalArgumentException("Could not find > ProductStore with ID [" + productStoreId > - + "] for tax calculation"); > + throw new IllegalArgumentException("Could not find > ProductStore with ID [" + productStoreId + "] for tax calculation"); > } > > if ("Y".equals(productStore.getString("showPricesWithVatTax"))) > { > Set<GenericValue> taxAuthoritySet = new HashSet<>(); > if (productStore.get("vatTaxAuthPartyId") == null) { > - List<GenericValue> taxAuthorityRawList = > EntityQuery.use(delegator).from("TaxAuthority") > - .where("taxAuthGeoId", > productStore.get("vatTaxAuthGeoId")).cache().queryList(); > + List<GenericValue> taxAuthorityRawList = > EntityQuery.use(delegator) > + .from("TaxAuthority") > + .where("taxAuthGeoId", > productStore.get("vatTaxAuthGeoId")) > + .cache() > + .queryList(); > taxAuthoritySet.addAll(taxAuthorityRawList); > } else { > GenericValue taxAuthority = > EntityQuery.use(delegator).from("TaxAuthority").where("taxAuthGeoId", > @@ -175,7 +181,9 @@ public class TaxAuthorityServices { > GenericValue facility = null; > try { > if (productStoreId != null) { > - productStore = EntityQuery.use(delegator).fro > m("ProductStore").where("productStoreId", productStoreId) > + productStore = EntityQuery.use(delegator) > + .from("ProductStore") > + .where("productStoreId", productStoreId) > .queryOne(); > } > if (facilityId != null) { > |
|
|
I see, well I suppose perhaps JIRA is also not the best place for
discussing "general" standards and coding practices. The ML is usually the first platform for these discussions. I also noted that Michael had concerns with the _value_ of the maximum width, not whether we should or should not have one (I could be mistaken, so apology in advance if I misread). I think either way it is worth a good discussion because even if 80 characters is too short let's say, then 800 characters might be too long! It's good to have a standard which unifies us I think. On Sat, Dec 16, 2017 at 6:14 PM, Jacques Le Roux <[hidden email]> wrote: > Actually we had a good discussion in the related Jira > https://issues.apache.org/jira/browse/OFBIZ-9877 > > We agreed it's time to rediscuss this again and I agree/and-suggested-there > the dev ML is the best place for that > > Jacques > > > > Le 16/12/2017 à 14:27, Taher Alkhateeb a écrit : >> >> I think it is always better to respect limit width because it has many >> implications that affect other developers. >> >> Anyway, I do not think the right place to make arguments is in commit >> messages. A commit message is supposed to hold information on what was >> done, not a place to argue formatting conventions or what you'd like to >> do. >> Perhaps it is more appropriate to use the ML for that. >> >> On Dec 16, 2017 12:28 PM, <[hidden email]> wrote: >> >> Author: jleroux >> Date: Sat Dec 16 09:28:37 2017 >> New Revision: 1818378 >> >> URL: http://svn.apache.org/viewvc?rev=1818378&view=rev >> Log: >> No functional change, just trivial formatting >> >> I began to reformat this supposed refactoring commit, but stopped and >> decided >> to rather discuss this in the related Jira. >> >> BTW automatically cutting lines at some narrow width does not make sense >> most of >> the time. >> See EntityQuery cases for instance, are they not easier to read when, IMO, >> rightly formatted, as I did here? >> >> Modified: >> ofbiz/ofbiz-framework/trunk/applications/accounting/src/main >> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java >> >> Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main >> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java >> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/app >> lications/accounting/src/main/java/org/apache/ofbiz/accounti >> >> ng/tax/TaxAuthorityServices.java?rev=1818378&r1=1818377&r2=1818378&view=diff >> ============================================================ >> ================== >> --- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main >> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java (original) >> +++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main >> /java/org/apache/ofbiz/accounting/tax/TaxAuthorityServices.java Sat Dec 16 >> 09:28:37 2017 >> @@ -63,8 +63,7 @@ public class TaxAuthorityServices { >> public static final int salestaxRounding = >> UtilNumber.getBigDecimalRoundingMode("salestax.rounding"); >> public static final String resource = "AccountingUiLabels"; >> >> - public static Map<String, Object> >> rateProductTaxCalcForDisplay(DispatchContext >> dctx, >> - Map<String, ? extends Object> context) { >> + public static Map<String, Object> >> rateProductTaxCalcForDisplay(DispatchContext >> dctx, Map<String, ? extends Object> context) { >> Delegator delegator = dctx.getDelegator(); >> String productStoreId = (String) context.get("productStoreId"); >> String billToPartyId = (String) context.get("billToPartyId"); >> @@ -87,20 +86,27 @@ public class TaxAuthorityServices { >> } >> >> try { >> - GenericValue product = EntityQuery.use(delegator).fro >> m("Product").where("productId", productId).cache() >> + GenericValue product = EntityQuery.use(delegator) >> + .from("Product") >> + .where("productId", productId).cache() >> + .queryOne(); >> + GenericValue productStore = EntityQuery.use(delegator) >> + .from("ProductStore") >> + .where("productStoreId", productStoreId) >> + .cache() >> .queryOne(); >> - GenericValue productStore = EntityQuery.use(delegator).fro >> m("ProductStore").where("productStoreId", >> - productStoreId).cache().queryOne(); >> if (productStore == null) { >> - throw new IllegalArgumentException("Could not find >> ProductStore with ID [" + productStoreId >> - + "] for tax calculation"); >> + throw new IllegalArgumentException("Could not find >> ProductStore with ID [" + productStoreId + "] for tax calculation"); >> } >> >> if >> ("Y".equals(productStore.getString("showPricesWithVatTax"))) >> { >> Set<GenericValue> taxAuthoritySet = new HashSet<>(); >> if (productStore.get("vatTaxAuthPartyId") == null) { >> - List<GenericValue> taxAuthorityRawList = >> EntityQuery.use(delegator).from("TaxAuthority") >> - .where("taxAuthGeoId", >> productStore.get("vatTaxAuthGeoId")).cache().queryList(); >> + List<GenericValue> taxAuthorityRawList = >> EntityQuery.use(delegator) >> + .from("TaxAuthority") >> + .where("taxAuthGeoId", >> productStore.get("vatTaxAuthGeoId")) >> + .cache() >> + .queryList(); >> taxAuthoritySet.addAll(taxAuthorityRawList); >> } else { >> GenericValue taxAuthority = >> EntityQuery.use(delegator).from("TaxAuthority").where("taxAuthGeoId", >> @@ -175,7 +181,9 @@ public class TaxAuthorityServices { >> GenericValue facility = null; >> try { >> if (productStoreId != null) { >> - productStore = EntityQuery.use(delegator).fro >> m("ProductStore").where("productStoreId", productStoreId) >> + productStore = EntityQuery.use(delegator) >> + .from("ProductStore") >> + .where("productStoreId", productStoreId) >> .queryOne(); >> } >> if (facilityId != null) { >> > |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |