search

buvinghausen / TaskTupleAwaiter

Async helper library to allow leveraging the new ValueTuple data types in C# 7.0
MIT License
71 stars 10 forks source link

Codegenerator tool #16

Closed taori closed 1 year ago

taori commented 5 years ago

@jnm2 feel free to verify. already worked on my machine.

taori commented 5 years ago

This would fix #14

taori commented 5 years ago

Looks like there is a CI fix required for this, unless you want to integrate tools into the source solution. Fixing CI would probably be better though.

taori commented 5 years ago

10 needs some updating

jnm2 commented 5 years ago

Looks like there is a CI fix required for this, unless you want to integrate tools into the source solution. Fixing CI would probably be better though.

Yes. This is all I can do until @buvinghausen is free to help: https://github.com/buvinghausen/TaskTupleAwaiter/issues/10#issuecomment-457943705

I can merge in spite of the build failure, but we should hear from Brian before too long.

taori commented 5 years ago

I diffed the tool output with arity 10 against our current source. I don't mind a little reformatting, but I ideally it shouldn't have to change that much:

jnm2@417b117?w=1

Is it ok the way it is now? Mind you that most of the whitespaces formatting is done by roslyns formatter, not me, so changing it would be a manual effort. Perhaps the best route would be executing the tests on it and make a "whitespace changes only" commit.

That's usually how i flag commits at my dayjob, to make sure a commit can be ignored if someone is looking for bug sources within the commit history.

jnm2 commented 5 years ago

Perhaps the best route would be executing the tests on it and make a "whitespace changes only" commit.

Yes, I'm a proponent of this approach.

Is it ok the way it is now?

I don't want the tool to change whitespace unless it's a change we would have made if we never had the tool. The added blank lines shouldn't be there, and the removed blank lines should still be there. Brian likes tabs. I like spaces, but I don't mind tabs so long as everything in the repo is consistent and there is an .editorconfig so that no one really has to care most of the time. Long story short: if TaskTupleExtensions.cs is going to change to spaces, I would want the entire repo to do that. Because I'd want to wait for Brian's input as the repo owner, can we make that a separate discussion and for now, either tell the formatter to follow the repo .editorconfig or else generate the code without needing it to be (re)formatted?

https://github.com/jnm2/TaskTupleAwaiter/commit/6aeb6b5073ed4f026872142950e42aee29e336ad?w=1

taori commented 5 years ago

Perhaps the best route would be executing the tests on it and make a "whitespace changes only" commit.

Yes, I'm a proponent of this approach.

Is it ok the way it is now?

I don't want the tool to change whitespace unless it's a change we would have made if we never had the tool. The added blank lines shouldn't be there, and the removed blank lines should still be there. Brian likes tabs. I like spaces, but I don't mind tabs so long as everything in the repo is consistent and there is an .editorconfig so that no one really has to care most of the time. Long story short: if TaskTupleExtensions.cs is going to change to spaces, I would want the entire repo to do that. Because I'd want to wait for Brian's input as the repo owner, can we make that a separate discussion and either tell the formatter to follow the repo .editorconfig or else generate the code without needing it to be (re)formatted?

jnm2@6aeb6b5?w=1

I see - makes sense. Perhaps you could try --format false and see how close the output is to the original then? I've made this a switch, so whoever invokes the tool, can decide if he wants the code untouched or formatted. That way both needs could be satisfied.

jnm2 commented 5 years ago

Perhaps you could try --format false and see how close the output is to the original then?

Here's what that looks like:

https://github.com/jnm2/TaskTupleAwaiter/commit/286ae3585a432632afe8efd07f8e6294e67b07ab

There are extra blank lines and other changes. I was picturing the tool fitting what we would have done by hand, rather than making us fit the tool.

jnm2 commented 5 years ago

(When working on the tool, can you check against the git diff of the output? It'll be less time consuming for you to do that live with the path CLI argument than for me to keep pushing a branch with only that change.)

taori commented 5 years ago

(When working on the tool, can you check against the git diff of the output? It'll be less time consuming for you to do that live with the path CLI argument than for me to keep pushing a branch with only that change.)

Did that now. The old source is not aligned the same every time so there is no way to make it match the old code file, unless it is aligned with ctrl k + d once. However you should just set up .editorconfig the way you want it once. Then you can just run the tool and format through IDE and it will be whatever you want the format to be anyway. that would cover all formatting issues i think.