-
Notifications
You must be signed in to change notification settings - Fork 132
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
Add more useful comments in spago.dhall #778
Conversation
dhall format no longer removes non-toplevel comments
- Remove link to package sets project - change 'project' to 'documentation' for spago - Change dhall link to the Language Tour
- add spaces between brackets and its content
I've addressed your comments. |
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.
Thank you, this looks great!
It looks like CI is failing because of some fixtures needing an update, but otherwise we're good to go 👏
When trying to build/test this locally, I ran into issues due to not having built a file for docs-search. The contributing file should be updated to clarify how to do this to make things easier for people. |
Latest commit fixes fixtures. Not sure if I got every one of them, but I think I did. |
Great, thank you! |
@JordanMartinez that's a great point - In the contributing file we do instruct to run |
@f-f Ah... No, it's clear enough. I just missed it because I skimmed it too quickly. I saw |
Description of the change
Fixes #708
Checklist:
README
P.S.: the above checks are not compulsory to get a change merged, so you may skip them. However, taking care of them will result in less work for the maintainers and will be much appreciated 😊