Conversation
…Filesystem methods
|
I also added checks to create the directory path if it does not exist. See here for discussion: #100 (comment) |
src/Testura.Code/Saver/CodeSaver.cs
Outdated
|
|
||
| private void EnsurePathExists(string filePath) | ||
| { | ||
| var fi = new FileInfo(filePath); |
There was a problem hiding this comment.
Minor thing but should maybe use the full name for FileInfo variables? So var fileInfo = new FileInfo(path).
I just think it's easier to read and understand when we use clear names for a variable (which may sound a bit like hypocrisy as I named CompilationUnitSyntax cu...). But it's more about taste and if you prefer fi we can stick with that.
There was a problem hiding this comment.
Sounds good! I am always for more 'explicit' style of programming, even if its more verbose - its easier to understand.
Haha, yea we all get lazy at times, thats why I use fi, its just laziness haha. I'll make the change now
There was a problem hiding this comment.
Done! I changed a few just to make them more explicit. I'd love to do this everywhere in the code eventually.
I'd also love to add more constructors and such so we can avoid using null as a parameter Such as this example:
// I'd love to remove the null, so the developer can just skip the 'WithBody' part and get the same result
var builder = new MethodBuilder("MyMethod").WithBody(null).Build()It will create this:
void MyMethod();I won't do that in this PR - but I just wanted to mention it as its related to the topic of being more explicit
There was a problem hiding this comment.
Sounds like a good idea to remove withbody requirment. Approved the PR as well.
Yeah it seems like a good thing to add as well. |
2 participants
here is the PR for #100
My editor formatted the
.editorconfigfiles, I don't think its a problem, but let me know if you'd like me to make any changes.Thanks!