[
https://issues.apache.org/jira/browse/OFBIZ-2668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12740441#action_12740441
]
Vikas Mayur commented on OFBIZ-2668:
------------------------------------
Hi Ahmed,
First of all thanks for working through these issues. This is your second patch
that I come across and its a good start though I found few issues. I know that
you are working on some Jira issues as is obvious from the mailing list
notification so going forward I thought to share few things with you. I hope
you would keep in mind the best practices before working on any issues. This
way you would save a lot of time of the reviewer and committer of the patch and
being said that this will also increase the chances of your patch being
committed to OFBiz sooner than expected.
Here is brief summary about the issues in your patch.
1. Never use tabs (tab key) in your code, instead use spaces (space bar).
2. Avoid unnecessary changes.
For example, there is no need to add ?if_exists function to
storeSurvey.productStoreSurveyId since productStoreSurveyId is the primary key
for this generic value and this will never be empty.
{code}
<input type= "hidden" name= "productStoreSurveyId" value=
"${(storeSurvey.productStoreSurveyId)?if_exists}">
{code}
Though adding this will not harm but it is just extra coding and should be
avoided.
3. Instead of defining a row counter variable, you can use the FTL built in
_index.
4. This change was also not required.
{code}
- <td>${storeSurvey.fromDate?string}</td>
+ <td>${storeSurvey.fromDate?if_exists?string}</td>
{code}
Looking forward to see more and more of your involvement with project.
Thanks,
Vikas
> Delete Store survey
> --------------------
>
> Key: OFBIZ-2668
> URL: https://issues.apache.org/jira/browse/OFBIZ-2668
> Project: OFBiz
> Issue Type: Sub-task
> Components: product
> Affects Versions: Release Branch 9.04, SVN trunk
> Environment: Ubuntu Linux 9.04, ofbiz trunk, revision -780597
> Reporter: Kojo Gambrah
> Assignee: Vikas Mayur
> Priority: Minor
> Fix For: Release Branch 9.04, SVN trunk
>
> Attachments: DeleteStoreSurvey.patch
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
