Skip to content
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

refactor README and installation instructions #2669

Merged
merged 23 commits into from
Nov 15, 2023

Conversation

joelberkeley
Copy link
Contributor

@joelberkeley joelberkeley commented Sep 19, 2022

I've been looking at doing a good overhaul of the docs, to

  • make it easier for newcomers
  • reduce duplication

and wanted to get early feedback. Do be completely honest. Is this helpful? I have an idea to work on the docs much more widely, so the approach here (short, to the point) is as important as the content.

Remaining tasks

  • mention The JavaScript codegen uses the new BigInt, hence Node.js 10.4 or higher is required. somewhere
  • how to include the videos in the README?
  • ensure line breaks at 80 ... is this necessary? CI only requires breaks at 400.
  • should we mention any specific installation instructions in the README? pack? from source?
  • should we summarise the main things new to Idris2 in the README?


For full installation instructions, see [INSTALL.md](INSTALL.md). Briefly, if
you have Chez Scheme installed, with the executable name `chez`, type:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the default "install from sources" stuff because I don't think most people will install from sources. Is that accurate?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think having a tl;dr for installing from sources is bad, due to the stuff I mentioned in my other comment.

README.md Show resolved Hide resolved
way to do so. Just select one good first issue and ask about it on the [Discord](https://discord.gg/UX68fDs2jc) channel.

Talks
-----
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd move these to andre's new Resources page on the wiki, and link that here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now added to resources

INSTALL.md Outdated
- [Installing from source](#installing-from-source)
- [Installing from a package manager](#installing-from-a-package-manager)
- [Install from a package manager](#install-from-a-package-manager)
- [Install from source](#install-from-source)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've assumed that most people won't want to install from sources

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure about this assumption. Installing from source is afaik still the only way that works regardless of what OS people are using. And Idris(2) is a research project, so I would assume the opposite: that most people install from the sources since they might want (or need) to modify them.

Copy link
Contributor Author

@joelberkeley joelberkeley Sep 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Installing from source is afaik still the only way that works regardless of what OS people are using

that's an interesting point

And Idris(2) is a research project, so I would assume the opposite: that most people install from the sources since they might want (or need) to modify them.

interesting. I got my impression from asking on discord. I guess much of my reasoning for my changes has been to make it easier for people new to Idris to dive in, i.e. emphasising growing the community. Also from my experience where I've had to add things missing from the stdlib, or wrap things with bugs, but never had to modify them

```sh
nix profile install github:idris-lang/Idris2
```

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that once you are making installation instructions more accessible, then maybe, installation through pack should be mentioned here also. This is for now a very good alternative for installation, especially if one wants to just use the latest Idris, especially with lots of ready libraries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

This is great, thank you! But I'm afraid adding "Make sure that $HOME/.pack/bin is on your PATH and takes precedence over the bin folder(s) (if any) where existing versions of Idris2 are already installed" from this INSTALL.md is also important, or else this text could be understood as "run bash -c ... and idris2 command becomes usable", where setting PATH is important too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pack instructions have become more detailed, so I've just decided to link to them instead

Copy link
Collaborator

@CodingCellist CodingCellist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit cautious of changing the README, since it's the first thing people see on the repo. But generally, this looks okay. I've left some comments on the files : )

(Minor nitpick: I'd keep the headings in present tense. They describe an action : ) )

INSTALL.md Outdated
- [Installing from source](#installing-from-source)
- [Installing from a package manager](#installing-from-a-package-manager)
- [Install from a package manager](#install-from-a-package-manager)
- [Install from source](#install-from-source)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure about this assumption. Installing from source is afaik still the only way that works regardless of what OS people are using. And Idris(2) is a research project, so I would assume the opposite: that most people install from the sources since they might want (or need) to modify them.

INSTALL.md Outdated Show resolved Hide resolved
README.md Outdated

[![Documentation Status](https://readthedocs.org/projects/idris2/badge/?version=latest)](https://idris2.readthedocs.io/en/latest/?badge=latest)
[![Build Status](https://github.com/idris-lang/Idris2/actions/workflows/ci-idris2.yml/badge.svg)](https://github.com/idris-lang/Idris2/actions/workflows/ci-idris2.yml)

[Idris 2](https://idris-lang.org/) is a purely functional programming language
with first class types.
[Idris 2](https://idris-lang.org/) is a purely functional programming language with first class types.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line (and the lines in general) are split at 80 chars to keep plaintext/un-rendered readability. Please restore this : )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done (though I may forget as I make more changes)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is curious. CI only complains when lines go over 400 chars. Why so?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the main remaining task

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok i think this is done


For full installation instructions, see [INSTALL.md](INSTALL.md). Briefly, if
you have Chez Scheme installed, with the executable name `chez`, type:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think having a tl;dr for installing from sources is bad, due to the stuff I mentioned in my other comment.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Comment on lines -112 to -119

[![Edwin Brady Tells Us What's New in Idris 2 (Berlin Functional Programming Group)](https://img.youtube.com/vi/nbClauMCeds/0.jpg)](https://www.youtube.com/watch?v=nbClauMCeds "Edwin Brady Tells Us What's New in Idris 2 (Berlin Functional Programming Group)")

[![Scheme Workshop Keynote (ACM SIGPLAN)](https://img.youtube.com/vi/h9YAOaBWuIk/0.jpg)](https://www.youtube.com/watch?v=h9YAOaBWuIk "Scheme Workshop Keynote (ACM SIGPLAN)")

[![Idris 2 - Type-driven Development of Idris (Curry On - London 2019)](https://img.youtube.com/vi/DRq2NgeFcO0/0.jpg)](https://www.youtube.com/watch?v=DRq2NgeFcO0 "Idris 2 - Type-driven Development of Idris (Curry On - London 2019)")

[![Idris 2: Type-driven Development of Idris (Code Mesh LDN 18)](https://img.youtube.com/vi/mOtKD7ml0NU/0.jpg)](https://www.youtube.com/watch?v=mOtKD7ml0NU "Idris 2: Type-driven Development of Idris (Code Mesh LDN 18)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep these in the file, they're useful introductions and showcases for what Idris2 is and can do.

We should probably add the SPLV'20 playlist to the mix (https://www.youtube.com/watch?v=2pa3oRFNO8E&list=PLmYPUe8PWHKqBRJfwBr4qga7WIs7r60Ql)

Copy link
Contributor Author

@joelberkeley joelberkeley Sep 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I do think they should appear differently. I feel we could communicate what's in each video much more effectively. The thumbnails are visually very busy and low-res even when they do show the content

Regarding useful introductions, should we also mention the official tutorials etc for those who prefer to read than listen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's how I presented them in the Resources wiki. It's another possibility

https://github.com/idris-lang/Idris2/wiki/2-%5BCommunity%5D-Resources#learning

@joelberkeley
Copy link
Contributor Author

joelberkeley commented Sep 21, 2022

I'm a bit cautious of changing the README, since it's the first thing people see on the repo. But generally, this looks okay. I've left some comments on the files : )

Indeed. That's why I want to get good feedback. I haven't made any attempt to be conservative in my changes.

(Minor nitpick: I'd keep the headings in present tense. They describe an action : ) )

they are in the present tense (Present simple rather than Present continuous). Is that OK?

@joelberkeley
Copy link
Contributor Author

I'm a bit cautious of changing the README, since it's the first thing people see on the repo

btw this is precisely why I'm working on it - because it matters

@CodingCellist CodingCellist added documentation Improvements or additions to documentation enhancement labels Oct 6, 2022
@CodingCellist CodingCellist mentioned this pull request Oct 10, 2022
@joelberkeley joelberkeley force-pushed the documentation branch 3 times, most recently from 3654da5 to 827502d Compare November 5, 2022 22:07
@joelberkeley joelberkeley marked this pull request as ready for review November 10, 2023 13:45
@@ -8,6 +8,7 @@ output an HTML file will also generate a basic HTML document with the
generated code inside a ``<script>`` tag; the other distinction is on the ffi
that will be explained below.

**Note**: The JavaScript codegen uses the new BigInt, hence Node.js 10.4 or higher is required.
Copy link
Contributor Author

@joelberkeley joelberkeley Nov 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved here since @dunhamsteve mentioned

anything prior to node 10 hit end of life by 2019, so I'm not sure that bit is relevant anymore. BigInt is widely supported, but the BigInt requirement might be relevant to someone targeting older browsers or an embedded javascript with more limited support.

and SlayerOfTheBad mentioned

Someone targeting older browsers will likely polyfill any of their JS before serving it anyways though

Copy link
Collaborator

@CodingCellist CodingCellist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of suggestions, but overall lgtm! : D

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • A link to the Map of the Source Code under the contributing resources might be good? : )
  • And I'd link the talks as well somewhere. IIRC they're now on the wiki, so just having a link to that : )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

README.md Outdated Show resolved Hide resolved
Co-authored-by: CodingCellist <[email protected]>
Copy link
Collaborator

@CodingCellist CodingCellist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

In terms of the 2 unticked list items, I think it's okay to just have the link to the "What's changed since Idris 1"-docs. And pointing to INSTALL.md for install instructions, while ever so slightly less ergonomic, is the easiest to maintain / least likely to get out of sync method. Generally, I think both are good as they currently stand : )

@joelberkeley
Copy link
Contributor Author

@CodingCellist I don't have the permissions to merge this, do you?

@andrevidela
Copy link
Collaborator

I've got you

@andrevidela andrevidela merged commit d6aa70d into idris-lang:main Nov 15, 2023
3 checks passed
@joelberkeley joelberkeley deleted the documentation branch November 15, 2023 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants