I have not been in this meeting you guys had, and I do not know the other side of the coin, but based on what I see in this long paragraph, everything Dwight says makes sense to me.  In particular, I do not want him to waste time go over minor things twice.  Also, everybody: you should be grateful for the code reviews and for being asked to improve your code, and not feel it like a hindrance.  This helps you write better and more responsible code, which is good for yourself, and helps us all make better use of what you are doing.  As an example, it took me several hours to incorporate Traian's comments on the generic visitors and the semantics of quote/unquote.  I initially didn't like it that he asked me to change the code, and then also add comments everywhere (particularly comments in his own definition of Agent!  I will pay you that back, Traian, one day :)), but in the end I am very satisfied with the final result.  I became better, the code became better, everything is better.  So if I initially had any vague doubt whether enforcing this code review policy was a good idea, I am now 100% convinced that it is one of the best SE practices that we ever did in our group.  So I fully support it.

Grigore


________________________________
From: dwightguth [notifications@github.com]
Sent: Thursday, April 03, 2014 6:18 PM
To: kframework/k
Cc: Rosu, Grigore
Subject: Re: [k] Newparser firstdraft (#318)


In src/javasources/KTool/src/org/kframework/parser/concrete2/Parser.java:

> +
> +            if (stateEnd != key.stateEnd) return false;
> +            if (!stateCall.equals(key.stateCall)) return false;
> +
> +            return true;
> +        }
> +
> +        @Override
> +        public int hashCode() {
> +            int result = stateCall.key.hashCode();
> +            result = 31 * result + stateEnd;
> +            return result;
> +        }
> +    }
> +
> +    final Key key;


There seems to have been a miscommunication of expectations here, unfortunately. Radu asked to meet with me this afternoon as part of a discussion going over the code review, like we discussed. I was under the impression going in that the purpose of this meeting was for me to go over the code review which would have been mostly done, understand what questions you guys still had regarding the work I asked for, and make sure I didn't still have any outstanding considerations regarding documentation or other tasks that weren't complete yet. However, at the meeting, it eventually became apparent that the idea Radu had was for me to go over the entire code again and look for small changes that had not been made yet and help him make them. The comments you see were made before I fully realized that fact. Unfortunately I have neither the time nor the resources to sit in a meeting and look for all the places that a change I requested was not made, when you are already aware of your concerns and the state of the code review and can communicate to me these questions electronically. If you want to meet tomorrow, I need you to collect an exhaustive list of questions regarding every single change I requested which has not been made yet, and have it ready at the meeting. I also need you to have made every single change you do not have concerns about which are significant in nature. Otherwise my time meeting will not be productive and I will be forced to postpone the meeting again until you are prepared for it. I am in a position where my time is very valuable because I am a bottleneck through which much of the productivity of the group has to pass. This means I cannot afford to spend time working on things I have already done, or which are trivial and I asked you to do. That means I also need you to make the code style changes that I requested where purely syntactic considerations make the code hard to read.If you're ready to meet on those terms by tomorrow, great, we can meet. but the code is /not/ ready for that to happen yet.
…
On Apr 3, 2014 5:56 PM, "mdmkolbe" <notifications@github.com> wrote: In src/javasources/KTool/src/org/kframework/parser/concrete2/Parser.java: > + > + if (stateEnd != key.stateEnd) return false; > + if (!stateCall.equals(key.stateCall)) return false; > + > + return true; > + } > + > + @Override<https://github.com/Override> > + public int hashCode() { > + int result = stateCall.key.hashCode(); > + result = 31 * result + stateEnd; > + return result; > + } > + } > + > + final Key key; We never claimed they were done. What is the point of comments like these? ... <#14529cec1f44a526_> On 04/03/2014 05:43 PM, dwightguth wrote: In src/javasources/KTool/src/org/kframework/parser/concrete2/Parser.java: > + > + if (stateEnd != key.stateEnd) return false; > + if (!stateCall.equals(key.stateCall)) return false; > + > + return true; > + } > + > + @Override<https://github.com/Override> <https://github.com/Override> > + public int hashCode() { > + int result = stateCall.key.hashCode(); > + result = 31 * result + stateEnd; > + return result; > + } > + } > + > + final Key key; not done -- Reply to this email directly or view it on GitHub < https://github.com/kframework/k/pull/318/files#r11276601>. -- Reply to this email directly or view it on GitHub<https://github.com/kframework/k/pull/318/files#r11277008> .

—
Reply to this email directly or view it on GitHub<https://github.com/kframework/k/pull/318/files#r11277637>.