-
Notifications
You must be signed in to change notification settings - Fork 843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make snapshot: a synonym for resolver #4341
Conversation
At this point I've added unit test |
@snoyberg can you review my current code? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Add a line to documentation and then it's ready to merge.
thanks
Agreed with @mihaimaruseac, thanks! |
@snoyberg as you mentioned in #4256
I'm not quite sure what the Also, there are multiple mentions of Config.hsstack/src/Stack/Types/Config.hs Line 625 in cc7e6cb
I'm not sure that if I rename it to "snapshot" everything will work fine. I can delete it without any test failWhat should I do? Other filesConfigCmd.hs What should I do with them? |
For snapshots files, this is what's known as "custom snapshots" and lives in the pantry part of the codebase. See: https://github.com/commercialhaskell/stack/blob/master/doc/pantry.md I believe that, for now, we should continue to use
Does that make sense? |
Partially, I still have a lot of questions 😃
|
|
Both Travis and AppVeyor builds has failed due to timeout, I don't know how to fix the issue |
Looks like it passed this time. Even the Travis job that "failed" actually succeeded, it just didn't complete the cache upload. So I believe this is ready for final review. @mihaimaruseac did you want to do that, or would you like me to? |
Thanks |
@mihaimaruseac thx for the reviews This was my first code contribution to an open source project ever, I'm proud of myself 😄 I would be able to write better code for Stack if you guys would point on some of my weak points so I could work on them.
Thx! |
I've never actually received that question on open source work before, I really appreciate your interest. I'll give you a big communication strength: you pointed out when you were delayed, and then quite clearly pointed out problems you were having. It made it very easy to assist you. I'm honestly very impressed with the code, and the fact that you included tests: many contributors don't do that. The only thing I'd recommend looking into is applicative parsers for CLI flags, so that you could more easily add I'd be very happy if you're interested in working on more issues. If you want any recommendations of good ones to onboard with, let me know. |
I second @snoyberg, this is the first time I see this, congrats. From my point of view, all went perfect in handling this |
Fixing #4256