Update Contributors libraries according to library guidelines

thomashoneyman commented 3 years ago

@milesfrain, @JordanMartinez, and I have been undertaking an effort to ensure all Contributors libraries meet the Library Guidelines, with the ultimate goal of ensuring these libraries are documented, tested, and welcoming to users as well as potential contributors.

Note: If you are interested in helping maintain any particular libraries, make sure to watch the repository.

This issue tracks progress on at least ensuring the libraries share the same structure, have CI via GitHub Actions, have their latest version published to Pursuit, use Spago for local development, and so on.

Checklist

Please see the library update checklist for the steps to take to update any library in this list.

Initial Library List [ Completed! ]

This list of libraries is our test case to get a feel for how we should update the remaining libraries.

Batch 1 [ Completed! ]

This is the first set of libraries to update, which are a bit less-used in case we hit any extra snags.

Batch 2 [ Completed! ]

Next, we'll update these more-used libraries:

Batch 3 [ Completed! ]

Finally, these libraries will take more care and should be left for last:

thomashoneyman commented 3 years ago

Edit: the checklist has moved:

https://github.com/purescript-contrib/governance/blob/main/updater/UpdateChecklist.md

JordanMartinez commented 3 years ago

@thomashoneyman Minor typo here:

- git mv .github/pull_request_template .github/PULL_REQUEST_TEMPLATE.md
+ git mv .github/pull_request_template.md .github/PULL_REQUEST_TEMPLATE.md

Also, spago init should be spago init -C to remove comments by default.

I need further clarification on how to use the contrib-updater tool correctly in regards to the package name fields:

  --display-name STRING    How to render this library's name in .md files.
                           Default: '`package-name`'
  --title STRING           How to render this library's name in titles. Default:
                           'package-name'

Given I have package purescript-foo, what should display-name and title be? I assume it's this:

contrib-updater generate --display-name purescript-foo --title purescript-foo

However, you said purescript-uri should be URI in some cases. Based on the docs above, it wasn't clear to me when one should be used over the other.

I'll update this with more comments if I come across any.

JordanMartinez commented 3 years ago

I'm claiming purescript-freet and purescript-string-parsers.

thomashoneyman commented 3 years ago

One quick clarification — package names here are referring to the Spago package name, ie. freet (instead of purescript-freet).

Usually, I supply a capitalized version of the package name as the title — “FreeT” or “String Parsers” — and don’t supply a display name at all (I use the default, which is the package name in backticks).

I only use a custom display name for some libraries like Halogen which are normally referred to by their proper name (Halogen) instead of the package name in backticks.

Thanks for catching those typos! Currently on my phone but I’ll come back and update that later.

JordanMartinez commented 3 years ago

Usually, I supply a capitalized version of the package name as the title — “FreeT” or “String Parsers” — and don’t supply a display name at all (I use the default, which is the package name in backticks).

In other words, if the package is purescript-foo-bar-baz, you would use

contrib-updater generate --title "Foo Bar Baz" --maintainer thomashoneyman

To clarify, should we get our PR reviewed and approved by one of us before merging them in? Or just merge it as soon as it's done?

thomashoneyman commented 3 years ago

Yes, that’s what I would typically do. And yes I think they should be approved before merging.

thomashoneyman commented 3 years ago

I've updated to fix the typos and clean up the instructions a little bit.

JordanMartinez commented 3 years ago

Just documenting this:

The package name used in code snippets (for example, the installation instructions) should be the same as that used in the spago.dhall file (ie. package-name)
The package name used in README contents should be the same as that used in the spago.dhall file, with backticks (ie. package-name)
The package name used in README titles should be the same as that used in the spago.dhall file, converted to title case without hyphens (ie. Package Name)
If a package name needs to be rendered differently in the README files, then you can provide a different format with the --display-title and --display-name flags. For example, the freet library would title case as “Freet” but it really ought to be “FreeT”.

JordanMartinez commented 3 years ago

purescript-these is now done.

thomashoneyman commented 3 years ago

Could I get a review on https://github.com/purescript-contrib/purescript-form-urlencoded/pull/18?

JordanMartinez commented 3 years ago

Ah, sorry about that. I was wondering why you hadn't merged that yet, haha.

JordanMartinez commented 3 years ago

I'll take nullable next.

JordanMartinez commented 3 years ago

Taking purescript-options

JordanMartinez commented 3 years ago

Also taking purescript-http-methods

thomashoneyman commented 3 years ago

Due to #19 and #17, let's hold off on doing any further libraries at the moment.

thomashoneyman commented 3 years ago

OK! We can now continue working on the remainder of batch 1.

JordanMartinez commented 3 years ago

Sweet!

thomashoneyman commented 3 years ago

@maxdeviant took care of media-types (thank you!).

maxdeviant commented 3 years ago

purescript-now was completed in https://github.com/purescript-contrib/purescript-now/pull/14.

thomashoneyman commented 3 years ago

The remainder of Batch 1 is now complete. Please hold off on updating any libraries from Batch 2 until I have a chance to update their admin settings!

thomashoneyman commented 3 years ago

OK -- Batch 2 is now ready to go for anyone who would like to take one on.

maxdeviant commented 3 years ago

I'll take purescript-arraybuffer-types.

thomashoneyman commented 3 years ago

Ready for review (I'll merge by EOD tomorrow unless anyone catches issues):

This (along with arraybuffer-types, handled by @maxdeviant) concludes Batch 2, and leaves us with just the last batch to go.

The automatic changelog generation, @JordanMartinez, is a really nice addition.

thomashoneyman commented 3 years ago

I've updated the settings for Batch 3 and any of those libraries are also available to take on.

maxdeviant commented 3 years ago

I noticed that some of the repositories in Batch 3 (purescript-aff, purescript-matryoshka) don't have any maintainers listed in the README.

Who should we pass as the --maintainer in these cases?

garyb commented 3 years ago

@natefaubion is definitely one for aff, it's mostly his baby at this point.

You can put me down for matryoshka. I'm not an expert in recursion schemes although I authored this, based on the Scala library, so it'd be nice if there are any experts who would have an interest in helping out too. Or non experts! But having someone who can assess suitability of proposed modifications would be great.

thomashoneyman commented 3 years ago

@maxdeviant if there are no maintainers, you can definitely ask here about it, and otherwise we generally default to @garyb and myself until we find someone more suitable.

JordanMartinez commented 3 years ago

Just wanted to say nice job @thomashoneyman on doing all of those library updates!

thomashoneyman commented 3 years ago

Could use reviews for a few libraries:

JordanMartinez commented 3 years ago

parsing and affjax have been reviewed. I'm not familiar with react, so I think someone else should review it.

thomashoneyman commented 3 years ago

Also completed:

thomashoneyman commented 3 years ago

We've done it! All libraries are complete, as far as pull requests go, so I'm going to close this issue. There are three libraries that haven't had their updates merged yet but I need to wait for reviews from other maintainers on those libraries before taking any more action. With that in mind I'm going to go ahead and close this issue.

maxdeviant commented 3 years ago

Party Parrot

purescript-contrib / governance

Update Contributors libraries according to library guidelines #13