Module & code documentation [#1996122]

Adding things like a readme and @file declarations. Very cool module :)

Comment	File	Size	Author
#19	ces-cleanup-1996122-19.patch	83.12 KB	hongpong
#16	ces-cleanup-1996122-16.patch	22.46 KB	esteve.badia
#15	ces-cleanup-1996122-15b.patch	26.53 KB	hongpong
#14	ces-cleanup-1996122-15.patch	23.97 KB	hongpong
#13	ces-cleanup-1996122-13.patch	5.26 KB	hongpong
#12	ces-cleanup-1996122-12.patch	47.47 KB	hongpong
#10	ces-cleanup-1996122-10.patch	76.14 KB	esteve.badia
#9	ces-cleanup-1996122-cleanup-9.patch	75.83 KB	hongpong
#8	ces-cleanup-1996122-8.patch	48.49 KB	hongpong
#7	ces-cleanup2-1996122-7.patch	37.61 KB	hongpong
#3	ces-cleanup-1996122-3.patch	49.13 KB	hongpong
#1	ces-docs-1-1996122.patch	3.97 KB	hongpong

Comments

Comment #1

hongpong commented 16 May 2013 at 09:31

Assigned:	hongpong	» Unassigned
Status:	Active	» Needs review
Issue tags:		+Documentation

Status	File	Size
new	ces-docs-1-1996122.patch	3.97 KB

Very simple readme file, lets see if this patch format works.

Comment #2

esteve.badia commented 16 May 2013 at 15:54

Applied! Thanks!

I'm not really familiar with the contributing workflow in Drupal community. I just applied the patch and commited the changes from my Drupal user. I guess that's all. :-)

Comment #3

hongpong commented 18 May 2013 at 10:38

Status	File	Size
new	ces-cleanup-1996122-3.patch	49.13 KB

Did a ton of coder cleaning in the bank forms inc file. I had some confusion about spacing (like, it seems to disagree with itself about some spacing rules). However many problems have been eliminated and tons of errors are gone now. I figure I should post a change this big for review, though I can merge it myself later.

I think two commits got in this patch and I may have to roll it again, if it doesn't merge.

Comment #4

esteve.badia commented 18 May 2013 at 16:43

Great job!

I've manually reviewed the patch and It seems all ok to me. Indeed the README.txt file also comes with this patch. The breaks after returns could be safely removed instead of adding a new empty line between them, but it isn't significant.

So, in my opinion, proceed to apply the patch to the master branch. Since I've modified also some lines of this file (adding an automatic new Drupal user for administering each new exchange), I prefer that you commit the changes so I can merge it to my local copy and commit. I promise I won't break the coding standards further :)

Comment #5

hongpong commented 18 May 2013 at 21:28

Status:

Needs review

» Reviewed & tested by the community

Ok, thanks for the approval!! I will merge it in promptly. Glad to help! I will toggle this bug # back to "Needs Review" when I have another patch along these same sorts of purposes. Will try to get it on there very shortly here today.

Comment #6

hongpong commented 18 May 2013 at 21:51

Status:	Reviewed & tested by the community	» Active
Issue tags:		+code review

OK after a little tweaking on my git environment, merges done for now. I think they are correct. I did one merge backwards but the result is OK.

Other code stuff: Before entering the project approval queue, We still need to change master branch into 7.x-1.x . See http://drupal.org/empty-git-master for Howto.

Also there is a need to sanitize some of the messages that are printed by the bank forms inc file - the coder warns that filter_xss() or similar should wrap the print statements. I didn't do any of that because I wasn't sure which was the correct sanitization filter. That was the primary security thing that popped up for me so far.

Comment #7

hongpong commented 19 May 2013 at 16:23

Status:

Active

» Needs review

Status	File	Size
new	ces-cleanup2-1996122-7.patch	37.61 KB

More coder cleanup stuff and comments for the bank install and bank logic PHP files. I can merge this in later after you look it over.

EDIT - got some rest, looked it over & merged it in.

Comment #8

hongpong commented 21 May 2013 at 11:17

Status	File	Size
new	ces-cleanup-1996122-8.patch	48.49 KB

I did more code review on the bank logic inc file. Calling it a night for now, added a lot of todo documentation stubs in order to satisfy the coder module. I don't think any logic is changed but I might have made a typo or 2. Also kind of winged it on some function descriptions. However overall I think this gets rid of many notices. If it looks good to you I will merge later Tues.

Comment #9

hongpong commented 22 May 2013 at 08:58

Status	File	Size
new	ces-cleanup-1996122-cleanup-9.patch	75.83 KB

More cleanup on the other bank files as well, the pages and theme inc files. I hope it's all right to just add little TODO doc stubs in order to keep coder happy. I saw the comments that more work is needed on these, but figure no harm in a little more code cleanup.

Comment #10

esteve.badia commented 22 May 2013 at 18:22

Status:

Needs review

» Reviewed & tested by the community

Status	File	Size
new	ces-cleanup-1996122-10.patch	76.14 KB

Reviewed and added some (and changed a bit a pair) function documentation comments.

Comment #11

hongpong commented 23 May 2013 at 07:47

Status:

Reviewed & tested by the community

» Active

Sweet, I applied it and merged to master. Plus I'm starting to understand this thing :)

I will keep trying to clean more files.

Comment #12

hongpong commented 23 May 2013 at 09:54

Status:

Active

» Needs review

Status	File	Size
new	ces-cleanup-1996122-12.patch	47.47 KB

This might be the last coder patch to run for now. All the files except those test loader files have at least been looked over. One other thing is that Coder wants interfaces to have "Interface" in their name.

Comment #13

hongpong commented 29 May 2013 at 06:30

Status	File	Size
new	ces-cleanup-1996122-13.patch	5.26 KB

I ran into some small issues when messing around. The default behavior when no exchanges or other data loaded needs to be tuned a little bit, and this helps.

Comment #14

hongpong commented 29 May 2013 at 08:11

Status	File	Size
new	ces-cleanup-1996122-15.patch	23.97 KB

Did a little more digging and was able to fix menu callbacks for edit/delete accounts. Also made a stub for the account deletion callback. Added more help text in exchange form. Added a default new exchange in Barcelona. Rearranged the menu array to make it a little more logical. Made the google map link non-mandatory.

Maybe don't want to have the default new exchange settings in there, but it helps move along testing better.

Comment #15

hongpong commented 29 May 2013 at 08:21

Status	File	Size
new	ces-cleanup-1996122-15b.patch	26.53 KB

... And just realized the same tweak should go on to the limits and transactions callbacks. Seems to be working nicely although the delete callback for limits still needs to be implemented :)

Comment #16

esteve.badia commented 30 May 2013 at 18:37

Status	File	Size
new	ces-cleanup-1996122-16.patch	22.46 KB

Reviewed! Great work! Only two points (updated patch)

file common.db.logic.inc lines 139-143: Generalized a bit the security check. But anyway a fatal error is not that bad in case a module table is really gone. This is a fatal case!
file ces_bank.module: changed '%' to '%ces_limitchain' and '%ces_transaction'. Thus Drupal automatically loads the object using respective load functions and passes it to the page callback. And those callbacks expect (or at least should expect, if there is an error it is at the callback functions side) loaded objects rather than identifiers.

Comment #17

esteve.badia commented 30 May 2013 at 18:39

Status:

Needs review

» Reviewed & tested by the community

Missed to set the status tag to reviewed :o

Comment #18

hongpong commented 31 May 2013 at 00:42

Status:

Reviewed & tested by the community

» Active

Alright it is merged in and committed hooray :) cc0ead4

Next I think I will look at coder compliance for private variables, or perhaps other error checking. I did not realize that about how the % callbacks work, thanks for the info! Or I am open to suggestions too definitely, no random task too small.

Comment #19

hongpong commented 31 May 2013 at 09:01

Status	File	Size
new	ces-cleanup-1996122-19.patch	83.12 KB

Here are a bunch of cleanups. More ambitiously, changing camelcase private variables and attempting to rename Transaction to TransactionInterface including in the function arguments and documentation. And AccountInterface as well. Adding Interface definitely makes it easier for me to understand.

Comment #20

hongpong commented 31 May 2013 at 09:03

Status:

Active

» Needs review

OOps. Flag to review.

Comment #21

esteve.badia commented 6 June 2013 at 16:17

This patch is conflicting with the current master branch. I'll try to apply it manually. I'll try not to get hurt with it ;-)

Comment #22

esteve.badia commented 6 June 2013 at 17:38

Status:

Needs review

» Needs work

Reviewed & commited :) Great work!

Comment #23

esteve.badia commented 6 June 2013 at 21:57

Status:

Needs work

» Active

Done a lot of code cleanup in order to pass the coder module exam. Now there isn't any coder notice except for several @see directives in ces_bank.logic.inc file that I don't understand. I believe they are correct.

Anyway, we can give it the OK. So perhaps we can start thinking about applying for drupal project. The tests (issue #1998104) are not exhaustive but they exist and test the most critical feature.

What do you think?

BTW, since the cleanup involves so many lines, I recommend to checkout a clean copy of the module from git to avoid merging conflicts.

Comment #24

hongpong commented 8 June 2013 at 09:50

This is great! It seems pretty much ready to go for a module application, it's certainly a lot more sophisticated than a lot of approved modules.

I don't think I formed those @see directives correctly - there is some other way they're supposed to be written that eludes me. Also the patches helps explain the serializer and entity in offers-wants to me - makes a fair bit more sense.

I copied the master branch to a new 7.x-1.x and pushed that to sandbox. It is supposed to be set up that way on official modules as the main branch (at least until D8).

Perhaps start writing up the module application here or on email & i might be able to help on that. So nifty!

Comment #25

esteve.badia commented 18 June 2013 at 18:24

There is still a lot of code cleanup to do. Now coder passes but there is pareview there with more hints:

http://ventral.org/pareview/httpgitdrupalorgsandboxesteve1367140git-7x-1x

Come on!

Comment #26

8 May 2014 at 17:30

Commit 0df9d84 on 7.x-1.x by esteve.badia:

Issue #1996122: Code cleanup following coder module.

Comment #27

12 May 2014 at 09:36

Commit a503676 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications by HongPong:
```
Issue #1996122 by HongPong: README.txt for documentation start
```
Commit c845a9b on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications by HongPong:
```
Issue #1996122 by HongPong: Bank form Code style cleanup via coder.
```
Commit b9123f8 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications by HongPong:
```
Issue #1996122 by HongPong: coder fixes for bank install and logic.
```
Commit cc0ead4 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications by HongPong:
```
Issue #1996122 by HongPong: Add form help text, some small bugs fixed...
```
Commit 74f69c2 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications authored by HongPong, committed by esteve.badia:
```
Issue #1996122 by HongPong: Cleanup. CamelCase and Interface issue in...
```
Commit 142eb10 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications by esteve.badia:
```
Issue #1996122 by esteve.badia: Pass coder exam.
```
Commit 5d8659d on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications by esteve.badia:
```
Issue #1996122 by esteve.badia: fixed naming conventions and security...
```
Commit 1c89294 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit d51bce0 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications by esteve.badia:
```
Issue #1996122 by esteve.badia: more code cleanup following pareview.
```
Commit c4ca7d6 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```

Comment #28

12 May 2014 at 11:05

Commit a503676 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop by HongPong:
```
Issue #1996122 by HongPong: README.txt for documentation start
```
Commit c845a9b on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop by HongPong:
```
Issue #1996122 by HongPong: Bank form Code style cleanup via coder.
```
Commit b9123f8 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop by HongPong:
```
Issue #1996122 by HongPong: coder fixes for bank install and logic.
```
Commit cc0ead4 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop by HongPong:
```
Issue #1996122 by HongPong: Add form help text, some small bugs fixed...
```
Commit 74f69c2 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop authored by HongPong, committed by esteve.badia:
```
Issue #1996122 by HongPong: Cleanup. CamelCase and Interface issue in...
```
Commit 142eb10 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: Pass coder exam.
```
Commit 5d8659d on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: fixed naming conventions and security...
```
Commit 1c89294 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit d51bce0 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: more code cleanup following pareview.
```
Commit c4ca7d6 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```

Comment #29

12 May 2014 at 11:23

Commit a503676 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop by HongPong:
```
Issue #1996122 by HongPong: README.txt for documentation start
```
Commit c845a9b on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop by HongPong:
```
Issue #1996122 by HongPong: Bank form Code style cleanup via coder.
```
Commit b9123f8 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop by HongPong:
```
Issue #1996122 by HongPong: coder fixes for bank install and logic.
```
Commit cc0ead4 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop by HongPong:
```
Issue #1996122 by HongPong: Add form help text, some small bugs fixed...
```
Commit 74f69c2 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop authored by HongPong, committed by esteve.badia:
```
Issue #1996122 by HongPong: Cleanup. CamelCase and Interface issue in...
```
Commit 142eb10 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: Pass coder exam.
```
Commit 5d8659d on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: fixed naming conventions and security...
```
Commit 1c89294 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit d51bce0 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: more code cleanup following pareview.
```
Commit c4ca7d6 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```

Comment #30

12 May 2014 at 11:27

Commit a503676 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop, 2215171-interop by HongPong:
```
Issue #1996122 by HongPong: README.txt for documentation start
```
Commit c845a9b on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop, 2215171-interop by HongPong:
```
Issue #1996122 by HongPong: Bank form Code style cleanup via coder.
```
Commit b9123f8 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop, 2215171-interop by HongPong:
```
Issue #1996122 by HongPong: coder fixes for bank install and logic.
```
Commit cc0ead4 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop, 2215171-interop by HongPong:
```
Issue #1996122 by HongPong: Add form help text, some small bugs fixed...
```
Commit 74f69c2 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop, 2215171-interop authored by HongPong, committed by esteve.badia:
```
Issue #1996122 by HongPong: Cleanup. CamelCase and Interface issue in...
```
Commit 142eb10 on master, 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop, 2215171-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: Pass coder exam.
```
Commit 5d8659d on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop, 2215171-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: fixed naming conventions and security...
```
Commit 1c89294 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop, 2215171-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit d51bce0 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop, 2215171-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: more code cleanup following pareview.
```
Commit c4ca7d6 on 7.x-1.x, 2216627-docs, 1916476-Import_data_from_CES, 1401884-Notifications, 2215167-interop, 2215169-interop, 2215171-interop by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```

Comment #31

12 May 2014 at 21:38

Commit 0df9d84 on 7.x-1.x, 2215169-interop, 2215171-interop by esteve.badia:
```
Issue #1996122: Code cleanup following coder module.
```

Comment #32

15 May 2014 at 10:18

Commit 0df9d84 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop by esteve.badia:
```
Issue #1996122: Code cleanup following coder module.
```

Comment #33

16 May 2014 at 07:06

Commit a503676 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications by HongPong:
```
Issue #1996122 by HongPong: README.txt for documentation start
```
Commit c845a9b on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications by HongPong:
```
Issue #1996122 by HongPong: Bank form Code style cleanup via coder.
```
Commit b9123f8 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications by HongPong:
```
Issue #1996122 by HongPong: coder fixes for bank install and logic.
```
Commit cc0ead4 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications by HongPong:
```
Issue #1996122 by HongPong: Add form help text, some small bugs fixed...
```
Commit 74f69c2 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications authored by HongPong, committed by esteve.badia:
```
Issue #1996122 by HongPong: Cleanup. CamelCase and Interface issue in...
```
Commit 142eb10 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications by esteve.badia:
```
Issue #1996122 by esteve.badia: Pass coder exam.
```
Commit 5d8659d on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications by esteve.badia:
```
Issue #1996122 by esteve.badia: fixed naming conventions and security...
```
Commit 1c89294 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit d51bce0 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications by esteve.badia:
```
Issue #1996122 by esteve.badia: more code cleanup following pareview.
```
Commit c4ca7d6 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit 0df9d84 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications by esteve.badia:
```
Issue #1996122: Code cleanup following coder module.
```

Comment #34

20 May 2014 at 14:26

Commit a503676 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b by HongPong:
```
Issue #1996122 by HongPong: README.txt for documentation start
```
Commit c845a9b on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b by HongPong:
```
Issue #1996122 by HongPong: Bank form Code style cleanup via coder.
```
Commit b9123f8 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b by HongPong:
```
Issue #1996122 by HongPong: coder fixes for bank install and logic.
```
Commit cc0ead4 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b by HongPong:
```
Issue #1996122 by HongPong: Add form help text, some small bugs fixed...
```
Commit 74f69c2 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b authored by HongPong, committed by esteve.badia:
```
Issue #1996122 by HongPong: Cleanup. CamelCase and Interface issue in...
```
Commit 142eb10 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b by esteve.badia:
```
Issue #1996122 by esteve.badia: Pass coder exam.
```
Commit 5d8659d on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b by esteve.badia:
```
Issue #1996122 by esteve.badia: fixed naming conventions and security...
```
Commit 1c89294 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit d51bce0 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b by esteve.badia:
```
Issue #1996122 by esteve.badia: more code cleanup following pareview.
```
Commit c4ca7d6 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit 0df9d84 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b by esteve.badia:
```
Issue #1996122: Code cleanup following coder module.
```

Comment #35

22 May 2014 at 19:28

Commit a503676 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact by HongPong:
```
Issue #1996122 by HongPong: README.txt for documentation start
```
Commit c845a9b on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact by HongPong:
```
Issue #1996122 by HongPong: Bank form Code style cleanup via coder.
```
Commit b9123f8 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact by HongPong:
```
Issue #1996122 by HongPong: coder fixes for bank install and logic.
```
Commit cc0ead4 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact by HongPong:
```
Issue #1996122 by HongPong: Add form help text, some small bugs fixed...
```
Commit 74f69c2 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact authored by HongPong, committed by esteve.badia:
```
Issue #1996122 by HongPong: Cleanup. CamelCase and Interface issue in...
```
Commit 142eb10 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact by esteve.badia:
```
Issue #1996122 by esteve.badia: Pass coder exam.
```
Commit 5d8659d on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact by esteve.badia:
```
Issue #1996122 by esteve.badia: fixed naming conventions and security...
```
Commit 1c89294 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit d51bce0 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact by esteve.badia:
```
Issue #1996122 by esteve.badia: more code cleanup following pareview.
```
Commit c4ca7d6 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit 0df9d84 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact by esteve.badia:
```
Issue #1996122: Code cleanup following coder module.
```

Comment #36

24 May 2014 at 16:44

Commit a503676 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers by HongPong:
```
Issue #1996122 by HongPong: README.txt for documentation start
```
Commit c845a9b on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers by HongPong:
```
Issue #1996122 by HongPong: Bank form Code style cleanup via coder.
```
Commit b9123f8 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers by HongPong:
```
Issue #1996122 by HongPong: coder fixes for bank install and logic.
```
Commit cc0ead4 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers by HongPong:
```
Issue #1996122 by HongPong: Add form help text, some small bugs fixed...
```
Commit 74f69c2 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers authored by HongPong, committed by esteve.badia:
```
Issue #1996122 by HongPong: Cleanup. CamelCase and Interface issue in...
```
Commit 142eb10 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers by esteve.badia:
```
Issue #1996122 by esteve.badia: Pass coder exam.
```
Commit 5d8659d on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers by esteve.badia:
```
Issue #1996122 by esteve.badia: fixed naming conventions and security...
```
Commit 1c89294 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit d51bce0 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers by esteve.badia:
```
Issue #1996122 by esteve.badia: more code cleanup following pareview.
```
Commit c4ca7d6 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit 0df9d84 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers by esteve.badia:
```
Issue #1996122: Code cleanup following coder module.
```

Comment #37

edumag commented 27 May 2014 at 06:52

Assigned:	Unassigned	» edumag
Issue summary:	View changes

Comment #38

27 May 2014 at 07:56

Commit a503676 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers, 1996122-Module_code_documentation by HongPong:
```
Issue #1996122 by HongPong: README.txt for documentation start
```
Commit c845a9b on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers, 1996122-Module_code_documentation by HongPong:
```
Issue #1996122 by HongPong: Bank form Code style cleanup via coder.
```
Commit b9123f8 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers, 1996122-Module_code_documentation by HongPong:
```
Issue #1996122 by HongPong: coder fixes for bank install and logic.
```
Commit cc0ead4 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers, 1996122-Module_code_documentation by HongPong:
```
Issue #1996122 by HongPong: Add form help text, some small bugs fixed...
```
Commit 74f69c2 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers, 1996122-Module_code_documentation authored by HongPong, committed by esteve.badia:
```
Issue #1996122 by HongPong: Cleanup. CamelCase and Interface issue in...
```
Commit 142eb10 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers, 1996122-Module_code_documentation by esteve.badia:
```
Issue #1996122 by esteve.badia: Pass coder exam.
```
Commit 5d8659d on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers, 1996122-Module_code_documentation by esteve.badia:
```
Issue #1996122 by esteve.badia: fixed naming conventions and security...
```
Commit 1c89294 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers, 1996122-Module_code_documentation by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit d51bce0 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers, 1996122-Module_code_documentation by esteve.badia:
```
Issue #1996122 by esteve.badia: more code cleanup following pareview.
```
Commit c4ca7d6 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers, 1996122-Module_code_documentation by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit 0df9d84 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 2271629-Comments_in_offers, 1996122-Module_code_documentation by esteve.badia:
```
Issue #1996122: Code cleanup following coder module.
```

Comment #39

edumag commented 27 May 2014 at 18:17

Status:

Active

» Closed (fixed)

Comment #40

edumag commented 27 May 2014 at 18:52

Code perfect review.

All files pass the standardized test.

It has also been grouped modules from doxygen, improving development documentation.

Comment #41

esteve.badia commented 28 May 2014 at 16:20

Priority:	Minor	» Major
Status:	Closed (fixed)	» Needs work

Thanks edumag for the fixes! But it seems that there are more issues to be solved before we close the issue:

Following: http://pareview.sh/pareview/httpgitdrupalorgsandboxesteve1367140git

I would omit all ces_xxx naming warnings, but there are a few functions that I think should be renamed: parse_wants, parse_trades...
I think we really should use the library contrib module to load jqplot and edit the installation instructions so that the administrator knows how to download that external library. It will be much more clean.
I think we should take a look also at the DrupalPractice output part. We should check at least the undefined and unused variables warnigs, and the single line error. If we remove the jqplot library the output will be more tractable.
The Codespell output deserves also some fixes.

So I reopen the issue.

With such a huge output it may be possible that we are missing something. You can re-run the review from the main page http://pareview.sh/ once the changes are pushed to the 7.x-1.x branch.

Once the pareview has a more attractive output, we should update our issue https://drupal.org/node/2267557 at the Drupal.org Project applications queue.

Comment #42

edumag commented 29 May 2014 at 05:29

I have found that if you ask pareview analyze all ices fails to function names, but if you analyze a module no.

I guess waiting ices_ces_bank_function() and not ces_bank_function().

Comment #43

xavip commented 29 May 2014 at 07:27

Hi,

2. I think we really should use the library contrib module to load jqplot and edit the installation instructions so that the administrator knows how to download that external library. It will be much more clean.

Problem about this is that jqPlot works with a core library and several separate plugins. Now ces_statistics is using this files:
jquery.jqplot.min.js
jqplot.dateAxisRenderer.min.js
jqplot.barRenderer.min.js
jqplot.canvasTextRenderer.min.js
jqplot.canvasAxisLabelRenderer.min.js
jqplot.canvasAxisTickRenderer.min.js
jqplot.categoryAxisRenderer.min.js
jqplot.pointLabels.min.js
jqplot.pieRenderer.min.js

Maybe we can reduce some of them, simplifing the graphics the module renders, but the installing procedure with library API will be a little bit complex. Do you know how can we simplify the installation?

Comment #44

edumag commented 29 May 2014 at 07:31

Now I'm looking to use the libraries module to fix it.

Wait for it to finish before doing anything.

Comment #45

esteve.badia commented 29 May 2014 at 10:11

(1) I think the ces_xxx name warnings from pareview are just false positives, so I wouldn't worry about that.

(2) Regarding the jqplot issue, I think the most easy installation process is to downolad the whole jqplot library, and not only the needed files. We could document it with the following command:

sites/all/libraries$ wget --no-check-certificate http://bitbucket.org/cleonello/jqplot/get/1.0.8.zip -O jqplot.zip
sites/all/libraries$ unzip jqplot.zip
sites/all/libraries$ mv cleonello-jqplot-xxxx jqplot

Then we have to access the needed files from our code at libraries/jqplot/src/xxx

Comment #46

edumag commented 29 May 2014 at 10:19

OK, i got

Comment #47

edumag commented 30 May 2014 at 18:00

Were corrected almost all errors.

And added on branch 7.x-1.x

Comment #48

edumag commented 30 May 2014 at 18:13

Status:

Needs work

» Closed (fixed)

Comment #49

12 June 2014 at 19:34

Commit a503676 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 1996122-Module_code_documentation, 2283781-accept-payments by HongPong:
```
Issue #1996122 by HongPong: README.txt for documentation start
```
Commit c845a9b on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 1996122-Module_code_documentation, 2283781-accept-payments by HongPong:
```
Issue #1996122 by HongPong: Bank form Code style cleanup via coder.
```
Commit b9123f8 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 1996122-Module_code_documentation, 2283781-accept-payments by HongPong:
```
Issue #1996122 by HongPong: coder fixes for bank install and logic.
```
Commit cc0ead4 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 1996122-Module_code_documentation, 2283781-accept-payments by HongPong:
```
Issue #1996122 by HongPong: Add form help text, some small bugs fixed...
```
Commit 74f69c2 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 1996122-Module_code_documentation, 2283781-accept-payments authored by HongPong, committed by esteve.badia:
```
Issue #1996122 by HongPong: Cleanup. CamelCase and Interface issue in...
```
Commit 142eb10 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 1996122-Module_code_documentation, 2283781-accept-payments by esteve.badia:
```
Issue #1996122 by esteve.badia: Pass coder exam.
```
Commit 5d8659d on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 1996122-Module_code_documentation, 2283781-accept-payments by esteve.badia:
```
Issue #1996122 by esteve.badia: fixed naming conventions and security...
```
Commit 1c89294 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 1996122-Module_code_documentation, 2283781-accept-payments by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit d51bce0 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 1996122-Module_code_documentation, 2283781-accept-payments by esteve.badia:
```
Issue #1996122 by esteve.badia: more code cleanup following pareview.
```
Commit c4ca7d6 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 1996122-Module_code_documentation, 2283781-accept-payments by esteve.badia:
```
Issue #1996122 by esteve.badia: code cleanup.
```
Commit 0df9d84 on 7.x-1.x, 2215167-interop, 2215169-interop, 2215171-interop, 1401884-Notifications_b, 2271149-opentransact, 1996122-Module_code_documentation, 2283781-accept-payments by esteve.badia:
```
Issue #1996122: Code cleanup following coder module.
```