Jump to content

Code Style (Revisited)


Akinari

Recommended Posts


  • Group:  Members
  • Topic Count:  15
  • Topics Per Day:  0.00
  • Content Count:  520
  • Reputation:   64
  • Joined:  11/19/11
  • Last Seen:  

Tabs is much easier and small file size.

Edited by Rejected
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  9
  • Topics Per Day:  0.00
  • Content Count:  554
  • Reputation:   70
  • Joined:  04/04/12
  • Last Seen:  

Bump for GreenBox /no1 ...:D

:meow:

  • Upvote 1
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  6
  • Topics Per Day:  0.00
  • Content Count:  134
  • Reputation:   35
  • Joined:  02/27/12
  • Last Seen:  

Any of the Core Devs feel free to take care of this as I'm out of commission this weekend as posted.

Thanks,

Cookie

I'm marking it as approved as the majority is in agree.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  8
  • Topics Per Day:  0.00
  • Content Count:  148
  • Reputation:   46
  • Joined:  11/02/11
  • Last Seen:  

Yes, now SVN Blame will blame the person who committed the TAB-to-spaces conversion. And when we revert this, SVN Blame will blame that person too! We'll have to do two "Blame previous revision" lookups to find out who actually added/edited a line!

The only way to prevent that is:

  1. undo r16968 by doing SVN Replace on the whole /trunk/src/ (replace it with revision 16967 of /trunk/src)
  2. have everyone re-commit r16969 to r16991 (23 commits)
  3. then do the proper AStyle conversion (4 spaces = 1 tab)
    - this will still cause converted lines to blame the wrong person
    (and when looking up SVN Blame you have to do "Blame previous revision")
    + but at least you only have to do Blame previous revision ONCE
    + and it will only be for the lines that were space-indented, then converted to tabs
    (I'm guessing less than 5% of the code? The majority of it was already indented with TABs prior to r16968.)

I'm FOR doing this.

We replace with the source from r16967, which was actually last-modified in r16966 (doesn't matter much).

Then we could do this:

• Either every one recommit their own commit, so blame will be properly placed which will take more time (depends on that dev's activity) which I strongly recommend we do this, regardless of revision number, at least we'd have blame righty (partially, source-related would be different but still blame the same person).

Also I'd go for keep commit AS IS even if they would be reverted or follow up'd after, for the sake of blame.

• Otherwise we take a single person to recommit everything and even though we'd have less "useless" revisions (or follow ups), blame would be broken. (which I don't like)

And since this code style thing is giving so much trouble (which I thought it wouldn't), personally I don't want this kind of headache anymore, so leave it be.

List of commits envolving source modifications and its authors:

http://trac.rathena.org/changeset/16988/rathena - malufett
http://trac.rathena.org/changeset/16987/rathena - ligtha
http://trac.rathena.org/changeset/16986/rathena - mkbu95
http://trac.rathena.org/changeset/16985/rathena - mkbu95
http://trac.rathena.org/changeset/16984/rathena - markzd
http://trac.rathena.org/changeset/16981/rathena - malufett
http://trac.rathena.org/changeset/16980/rathena - markzd
http://trac.rathena.org/changeset/16979/rathena    - markzd
http://trac.rathena.org/changeset/16977/rathena - akkarin
http://trac.rathena.org/changeset/16974/rathena - mkbu95
http://trac.rathena.org/changeset/16973/rathena - mkbu95
http://trac.rathena.org/changeset/16971/rathena - mkbu95
http://trac.rathena.org/changeset/16969/rathena - lighta

  • Upvote 2
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  6
  • Topics Per Day:  0.00
  • Content Count:  134
  • Reputation:   35
  • Joined:  02/27/12
  • Last Seen:  

I am starting to work on this now.

Complete.

Here's the diff, if someone want to check:

http://www.mediafire.com/download.php?peh69n9fbvadwr1

Patch the revertTo16967 before.

I pretended to commit it a little later, but not sure after talking to mkbu95.

It'd be good if we could make a meeting with everyone at irc.

  • Upvote 1
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  21
  • Topics Per Day:  0.00
  • Content Count:  326
  • Reputation:   19
  • Joined:  09/27/12
  • Last Seen:  

So this isn't a live update yet in the SVN?

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  6
  • Topics Per Day:  0.00
  • Content Count:  134
  • Reputation:   35
  • Joined:  02/27/12
  • Last Seen:  

Implemented at: 16992

We opted by replacing the src svn with the old version 16966.

And since this code style thing is giving so much trouble (which I thought it wouldn't), personally I don't want this kind of headache anymore, so leave it be.

We agreed.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  75
  • Topics Per Day:  0.02
  • Content Count:  2223
  • Reputation:   593
  • Joined:  10/26/11
  • Last Seen:  

The only way to prevent that is:

  1. undo r16968 by doing SVN Replace on the whole /trunk/src/ (replace it with revision 16967 of /trunk/src)
  2. have everyone re-commit r16969 to r16991 (13 commits to /trunk/src)
  3. then do the proper AStyle conversion (4 spaces = 1 tab)
    - this will still cause converted lines to blame the wrong person
    (and when looking up SVN Blame you have to do "Blame previous revision")
    + but at least you only have to do Blame previous revision ONCE
    + and it will only be for the lines that were space-indented, then converted to tabs
    (I'm guessing less than 5% of the code? The majority of it was already indented with TABs prior to r16968.)

Part 1 is done.

Part 2:

Edited by Brian
  • Upvote 1
Link to comment
Share on other sites

×
×
  • Create New...