Skip to content

Update according to Contributors library guidelines #53

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

Merged
merged 11 commits into from
Aug 29, 2020
Merged

Update according to Contributors library guidelines #53

merged 11 commits into from
Aug 29, 2020

Conversation

JordanMartinez
Copy link
Contributor

@JordanMartinez JordanMartinez commented Aug 28, 2020

This pull request is part of an effort to update and standardize the Contributors libraries according to the Library Guidelines. Specifically, it:

  1. Adjusts the files and repository structure according to the repository structure section of the guidelines, which includes standard pr templates, issue templates, CI in GitHub Actions, automatic stale issue management, ensures the project uses Spago, and so on.
  2. Updates the README and documentation according to the documentation section of the guidelines. This is a first step towards ensuring Contributors libraries have adequate module documentation, READMEs, a docs directory, and tests (even if just usage examples) in a test directory.
  3. Updates labels where relevant to help folks better sift through issues on this library and get started contributing.

This PR is the groundwork for followup efforts to ensure contributor libraries are kept up-to-date, documented, tested, and accessible to users and new contributors.

Copy link
Contributor

@thomashoneyman thomashoneyman left a comment

Choose a reason for hiding this comment

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

This is pretty good! I think we can remove the backups directory and I'd like to switch 'uri' for 'URI' in the titles and purescript-uri for uri as the displayed package name.

README.md Outdated
@@ -1,148 +1,43 @@
# purescript-uri
# uri
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm in favor of capitalizing packages in titles where appropriate (in this case, URI). In the tool that'd be --display-title URI.

Copy link
Contributor Author

@JordanMartinez JordanMartinez Aug 29, 2020

Choose a reason for hiding this comment

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

Ok. I'm going to run the following code to update this:

contrib-updater generate --title 'URI' --maintainer garyb --display-name '`uri`'

It wasn't clear to me what the difference between title and display-name was, nor our preference for how to use each.

Copy link
Contributor

Choose a reason for hiding this comment

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

That should be better documented. The short version:

  • display-title is used only in headers, when you might display the package in title case. Defaults to the package name, no backticks.
  • display-name is used when referring to the package informally, like "Thank you for contributing to ". Defaults to the package name, with backticks (so specifying --display-name '`uri`' is not necessary)

The package name itself is used in any installation / code instructions (but this is done automatically for you, with no option). Typically I'm only supplying the --display-title flag to capitalize the package and rely on the package name for the display name.


```
bower install purescript-uri
```
Copy link
Contributor

Choose a reason for hiding this comment

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

This file can be deleted.

docs/README.md Outdated
@@ -0,0 +1,134 @@
# uri Documentation

This directory contains documentation for `purescript-uri`. If you are interested in contributing new documentation, please read the [contributor guidelines](../.github/CONTRIBUTING.md) and [What Nobody Tells You About Documentation](https://documentation.divio.com) for help getting started.
Copy link
Contributor

Choose a reason for hiding this comment

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

Another place to rename the package name to just uri.

Along with these types, you'll want to define an options record that specifies how to parse and print URIs that look like this:

``` purescript
options ∷ Record (URIRefOptions UserInfo (HostPortPair Host Port) Path HierPath RelPath Query Fragment)
Copy link
Contributor

Choose a reason for hiding this comment

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

This hasn't been mentioned before, but I would like to standardize on no unicode symbols within contrib. I think they're perfectly fine in personal code, but it certainly seems standard in the community to use non-unicode characters (:: instead of here) and it means cotributors who don't use them don't have to learn how to create or copy-paste them. If you have a moment to glance through and replace them in the docs I think that would be useful.

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 think every place you would expect ::, Gary used . I think I prefer the unicode character because things line up better and forall is reduced to something much smaller. However, I realize that could confuse new people who aren't sure what that means.
Regardless, I think we should address this in a separate PR.

@JordanMartinez
Copy link
Contributor Author

I've addressed your comments.

@thomashoneyman thomashoneyman merged commit 9c92e65 into purescript-contrib:main Aug 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants