Closed GoogleCodeExporter closed 8 years ago
No objections in adding it to a tooltip. Once you are ready with the patch use
epgp/scripts/codereview.py to submit it for a codereview.
Thanks!
Original comment by evlogimenos
on 6 Jun 2009 at 10:16
I could see epgp/scripts/codereview.py in the folder but had no means of
running. Is
it a python script?
I am an addon developer and use SVN so what I did was to compile a patch using
Tortoise and have attached that here. You will see from the patch that there are
really minimal changes to make this work.
I wasn't able to test any of the other localisations so I didn't touch any
localisation file other than localization.enUS.lua I wasn't sure of your
localisation
policy so I thought it best to leave them alone this does of course mean that
those
other localisations are missing the 3 additional entries listed in the patch.
Original comment by ShammyLe...@gmail.com
on 20 Jul 2009 at 5:12
Attachments:
Perhaps that last comment wasn't clear. The attached patch file is fully
working in
enUS localisation. Hopefully you can incorporate these small tweaks into the
base code.
Original comment by ShammyLe...@gmail.com
on 20 Jul 2009 at 7:22
This is also very important to our guild.
Is this being added to a future epgp release? Or as a patch?
please let me know how to implement this.
Issue: Show GUILD RANK in EPGP Report
Original comment by Hobb...@gmail.com
on 21 Jul 2009 at 5:14
If you upload to codereview it would be a lot easier to review the patch. If
you are on windows you can install
python from http://www.python.org/download/releases/. 2.6.2 would do. Do not go
with the 3.x releases since
most python programs do not work with them.
Thanks!
Original comment by evlogimenos
on 21 Jul 2009 at 9:21
Second attempt to reply - reply on forum doesn't seem to have posted.
I installed python v2.6.2 on my Vista box, ran a command prompt and changed
into the
epgp/scripts folder. I then typed codereview.py to execute the code. I guessed
this
was what it was as I could see no readme to tell me what to do, and your
comment just
said to run that. However I got an error immediately saying ...
C:\Games\World of Warcraft\Interface\AddOns\epgp\scripts\codereview.py:37: Depre
cationWarning: the md5 module is deprecated; use hashlib instead
import md5
Got error status from ['svn', 'info']:
Should I have had some command line parameters to the script if so what? Are
there
any instructions for what to do with the python script? I couldn't find any.
The previously attached patch is REALLY REALLY small and I tried to ensure I
kept the
changes to a minimum and followed your coding standards. I'm happy to try again
with
codereview if you tell me how its meant to work please.
Original comment by ShammyLe...@gmail.com
on 22 Jul 2009 at 6:26
You need to install a command line svn client
(http://www.sliksvn.com/en/download)
and make sure it is in your path for codereview.py to work.
Original comment by evlogimenos
on 25 Jul 2009 at 5:14
[deleted comment]
[deleted comment]
Ok I've managed to post an issue and hopefully you can now see it. How exactly
is a
review conducted?
Original comment by ShammyLe...@gmail.com
on 26 Jul 2009 at 5:55
I replied in the review. As for the different svn client, I am unsure why it is
needed or
not. I have no windows machine so I do not know if tortoise svn has a command
line
executable for the codereview.py script to use.
The review on codereview is a lot easier to conduct than here in the bug
report. You
comment the code directly and discussion happens a lot more naturally. Most of
the
code we commit to the tree goes through a review, so do not feel that this is
an extra
burden for you. Apologies if this seems too much but really it makes life for
the users
a lot better :-)
Original comment by evlogimenos
on 27 Jul 2009 at 6:16
No its ok - I worked out how to do it. Sorry if I appeared too frustrated.
Original comment by ShammyLe...@gmail.com
on 27 Jul 2009 at 10:39
Somewhat insulting to say I needed "handholding" to setup python etc. When it
was you
that was utterly wrong about what was required.
All you needed to do was to say please upload the patch file to the review site
by
creating an issue. Instead you pointed me to a python script (NOT NEEDED) and a
command line SVN client (NOT NEEDED). If you had simply said to upload the
patch by
creating a new issue, which is what I eventually did this would have taken a
whole
lot less time.
I was offering to assist by adding functionality to your addon, instead you
have been
obstructive and condescending. I have 26 years programming experience and so am
well
aware of the issues, I simply hadn't used the googlecode website before and so
didn't
see why I needed python etc to upload a patch. It transpires you were completely
wrong and I didn't need python or a different svn client at all and so this only
added to the confusion.
You now want me to recode everything I've done to assist you. Sorry but why
should I,
if you want to provide this feature for your users you are perfectly capable of
adding it yourself. I've given you working code, if you want to tweak it to
suit your
own ideas about what works and what doesnt that is entirely your perogative.
However
to be honest I'm fed up of jumping through your needless hoops, with no end in
sight.
As I say the code works perfectly well for myself and others in my guild and
those
who have contacted me directly for the patch.
I was trying to help, all I got was obstruction and insults, I shall not try
again in
future.
2009/7/28 Alkis Evlogimenos ('Αλκης Ευλογημένος)
<alkis@evlogimenos.com>
The difference between that and doing the changes is that this patch is going to
take a lot longer to apply. Also, consider that you needed handholding to set
python
and svn up to send the review, then I spend time to do the review and you now
saying,
you don't care to complete it, it works for you so I should do it on my own.
That's a
terrible waste of time for me, I could have implemented this in less time than I
already invested in it. So the least you could do is finish the code.
2009/7/28 Alexander Bisset <shammylevva@googlemail.com>
Ok well fair enough, by all means change the patch to achieve that.
I am happy that it works as I have it and use it for my guild, if you want to
provide the functionality for others I'm sure you can adjust the code
accordingly.
--
Alkis
Original comment by ShammyLe...@gmail.com
on 30 Jul 2009 at 1:27
- I did not insult you at any point in our discussions. I am not sure why you
say
that.
- Apologies for sending you to the wrong direction with python, I forget there
is a
way to upload patches through the web interface since I never use it.
- I do not want you to recode everything. I want you to refactor some of the
code
because it is not where it should be.
- If you want to help, you have to respect other teams code styles and
processes.
Code style: you didn't respect even after I asked in the code review. I let it
go
because it is easy to fix. Process: you mock it with your response "do on your
own". - When you have no intention on working on a code review, please say that
from the
get go. When you ask the whys and why nots the reviewer spends quite a bit of
time to
explain why things need to change. If they just go to void you are
intentionally
wasting their time. Also your reaction: "oh you know what, do it yourself", is
insulting.
- You also seem to be getting very frustrated quite easily. Maybe you should
consider
anger management classes?
In any case I am sure your efforts were in good faith but sadly your methods
are not
compatible with ours. Thanks for your time!
Original comment by evlogimenos
on 30 Jul 2009 at 3:05
As a 3rd party observer, I am hoping that this enhancement gets put into the
final
add on. In the end, this is a very useful for a lot of guilds as many
(including
mine) partially tie loot bidding to rank. It would make our lives 1,000xs
easier if
rank would show up in the ML report.
I'm sure you are both talented programmers or add ons like this would never
happen.
Lets forget about the process and discuss solutions to make this happen.
Alkis, Did he give you enough code to complete the work on your own?
If so, do you have intentions of adding this feature in an update? (Timeline?)
maybe it makes some sense to post a guideline to other programmers on your
process
and the "suggested" way to post code changes/patches. If this is already out
there I
apologize. I am acting as a mediator in cyberspace LOL. I just hate to see two
talented guys working on the same project butt heads over something silly.
Original comment by Hobb...@gmail.com
on 30 Jul 2009 at 3:29
I had every intention of helping. However I had no idea of the level of effort
that
would take and you provide no information to people looking to assist on what is
involved in trying to help.
Normally when someone is working on an addon they take other peoples ideas and
code
it for themselves. If someone else goes to the hassle of providing a patch
(scroll up
to find it) they look at the patch and say ah ok I see how he did that. Not
quite to
my style but thanks I'll adapt it to fit in my addon.
What is NOT normal is to expect commercial level patch management and review
processes. You DID NOT SAY at the outset that was what you expected. You said
"use
epgp/scripts/codereview.py to submit it for a codereview." With no explanation
of
what you expected a code review to be.
To be clear my expectation was that you'd review the patch yourself see if it
was
viable in your addon and decide to incorporate it or tweak it to suit your
coding
practices this is 100% the norm with Wow addons. I'm the author of half a dozen
addons and participate widely in addon forums, and have never come across
anyone who
wanted anything other than the patch submitted.
I regret we have "butted-heads" this wasn't my intention, I was just frustrated
by
the way you seemed to be putting up obstacles at every stage to my
contribution. As
such I had no way of knowing if I ploughed ahead and did what you asked if
you'd just
come back with more obstacles and so I quickly came to the conclusion that I'd
done
more than enough to prove the concept and it was far far better to leave you to
get
on with it than to jump through more hoops with no information on where it
might end.
It is this lack of direction and over the top expectations that frustrated me
most.
I've consulted other developers and shown them this correspondence and the
overwhelming view is that you are expecting above and beyond what is reasonable
for a
simple little patch to a wow addon.
I believe I have tried to act in good faith, I supplied an idea, I supplied a
solution now its up to you to decide whether you implement that solution in a
manner
that suits your own working methods. To ask more of anyone who is volunteering
information is overkill. If you do expect this you should be totally upfront at
the
outset as if I'd known in advance I'd never have submitted a patch I'd simply
have
altered the code for my own purposes.
I just thought it better to share my work with a wider audience and to help
improve
your excellent product. I do regret any rudeness I would ask however that you
provide
clearer information to possible contributors in future so that you don't
irritate
more people who are trying to be helpful to the wider gaming community.
Original comment by ShammyLe...@gmail.com
on 30 Jul 2009 at 3:56
You really like to waste time do you? For a patch that takes 15 minutes to
write we
have a total toll of:
1) A review that took 15 minutes for you to write it and 15 minutes total for
me to
review it. 30 minutes
2) You ask other developers about it our communication medium. Say 5 developers
for 5 minutes each, another 25 minutes. Add the 5 minutes you spent as well, 30
minutes.
3) Continuing the discussion here for another 10 or so minutes for each of us,
another 20 minutes.
So in the end for a 15 minute worth of code we already spent 1h20 minutes of
engineering time. If you just made the changes it would have been committed by
now.
Original comment by evlogimenos
on 30 Jul 2009 at 5:16
Bah, sorry this came off a bit offensive. Didn't mean it that way, I am on a
rush. But rly
think about the time we spent on this already and if it is worth it.
Original comment by evlogimenos
on 30 Jul 2009 at 5:20
Fair enough, lets let it rest. I do hope you can see fit to adding the concept
of my
code in whatever manner to your excellent addon. Please keep up the good work
and
please allow willing contributors just to submit code in future that YOU
examine and
integrate, without expecting them to go over it all again too.
I look forward to seeing future improvements.
Original comment by ShammyLe...@gmail.com
on 31 Jul 2009 at 8:44
This is fixed in r1283.
Original comment by evlogimenos
on 1 Aug 2009 at 11:41
Original issue reported on code.google.com by
ShammyLe...@gmail.com
on 4 Jun 2009 at 2:27