Skip to content

Tidying up a bit #143

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 8 commits into from
Apr 9, 2020
Merged

Tidying up a bit #143

merged 8 commits into from
Apr 9, 2020

Conversation

hdgarrood
Copy link
Collaborator

@hdgarrood hdgarrood commented Apr 7, 2020

  • Consolidate the client and server READMEs into one file
  • Fix local development instructions in README
  • Remove some leftover backend stuff
  • Update the license year
  • Update the default main gist ID to one which uses gist.github.com/:gist-id urls, as expected by Allow gist navigation #140, so that the links work
  • Update "help" link to point to main repo readme, rather than gh-pages branch

This one uses `gist.github.com` links for the examples so that the links
work.
We probably won't be using this branch once we deploy the current
version, and everything is in the main repo README now anyway.
@hdgarrood
Copy link
Collaborator Author

cc @thomashoneyman @gabejohnson if either of you have time and would like to take a look at this?

README.md Outdated
- Response body: Either `{ js: "..." }` or `{ error: "..." }`
- Status code: 200 (success), even if compilation failed

The response does not use error codes, to make it easier to use the API from another domain using CORS.
Copy link
Member

Choose a reason for hiding this comment

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

I think in the context of a compilation service a "compiler error" would still be a success either way. So making it easier to do CORS is nice, but I wouldn't need that motivation here.


The response does not use error codes, to make it easier to use the API from another domain using CORS.
- **View Mode**: Control the view mode using the `view` parameter
Copy link
Member

Choose a reason for hiding this comment

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

We should probably include the default for each of these, ie.

Control the view mode using the `view` parameter.

- Options are: `code` | `output` | `both`
- Default is: `both`
- Example: `view=output` will only display the output

You could judge this for yourself by loading up the default Try PureScript page, but if you're say crafting a query string based off of this section in the readme it would be useful to see which options you can skip.

@thomashoneyman
Copy link
Member

I have a few comments (the suggestions can be applied automatically), but they aren't strictly necessary and this otherwise looks good to me.

@hdgarrood
Copy link
Collaborator Author

Thanks! I'm going to go ahead and merge this now just to keep things moving but please feel free to comment/open PRs if you have further suggestions.

@hdgarrood hdgarrood merged commit 1efb0fd into master Apr 9, 2020
@hdgarrood hdgarrood deleted the harry/consolidate-readmes branch April 9, 2020 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants