Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
docs: proof-reading the tutorial #2636
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: master
Are you sure you want to change the base?
docs: proof-reading the tutorial #2636
Changes from all commits
1c7de22
74f2b0f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Comma seems worthwhile, but not sure about the increase in legibility/intelligibility arising from the change in/of preposition, maybe going all out and writing
(possibly with knock-on consequences for the line-breaking/layout)
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.
Why was this paragraph removed?
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.
@Taneb well, two reasons.
One: it seemed a bit misplaced... Just below, there's detailed exposition of
Bundle re-exporting
.Two: it looks possibly outdated; talks about
record Semigroup
having a"repackaging function" semigroup which converts a Magma to a Semigroup
— meanwhile, what's actually there in the definition, is the opposite: a functionmagma
that projects the implied Magma out of the Semigroup.Perhaps I misunderstood; please confirm. I can recover the paragraph easily, it just didn't read to me as correct neither helpful.
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.
Actually, I think there's preferable solution to this problem, namely to rewrite the whole paragraph so as to explain (hopefully: better!) what's actually going on ... but for the unfortunate lack of distinction between a 'primitive'/'definitional'
field
and what elsewhere in PL would be called a 'manifest field', viz. the definitions made either explicitly (as with themagma
example at hand), or by being brought into scope by anopen
declaration (as withisMagma
on which it depends)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.
Suggest, therefore instead, as a drop-in replacement for the deleted paragraph (GitHub won't apparently let me add this as a
suggestion
, alas):Now, I fully expect colleagues to object to my rephrasing... ;-) which is why I haven't tried to push such a commit to this PR without further discussion!
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.
@Taneb @ulidtko if you're happy with the suggested changes, please say so, or else offer alternatives.
propose that we try to merge sooner rather than later?