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

Docs update: include react api, aragon app architecture & fixes #271

Merged
merged 9 commits into from Apr 3, 2019

Conversation

5 participants
@0xGabi
Copy link
Member

0xGabi commented Mar 29, 2019

closes aragon/hack#23

New changes to restructure aragonAPI section: aragon/hack#102

@0xGabi 0xGabi requested a review from 0x6431346e Mar 29, 2019

@0xGabi 0xGabi referenced this pull request Mar 30, 2019

Merged

aragonAPI docs restructure #127

4 of 4 tasks complete

@0xGabi 0xGabi added this to In progress in Aragon Mesh Team via automation Mar 31, 2019

@0xGabi 0xGabi moved this from In progress to Review in Aragon Mesh Team Mar 31, 2019

@0x6431346e
Copy link
Member

0x6431346e left a comment

Very neat!!

I wonder if we can use the syncing scripts from the hack repo to pull React hooks docs directly from packages/aragon-api-react/README.md instead of docs/HOOKS.md, so they're easier to update. 🤔

docs/APP.md Outdated
@@ -1,4 +1,4 @@
# App
# aragonJS - App API

This comment has been minimized.

Copy link
@sohkai

sohkai Apr 2, 2019

Member

Not sure if we should change the title to use aragonJS; see aragon/hack#127 (comment).

Maybe just "JavaScript — App API", etc. would work as headings?

This comment has been minimized.

Copy link
@bpierre

bpierre Apr 2, 2019

Member

Shouldn’t we use “aragonAPI” instead of “App API”, since it is the main module?

We could have:

  • aragonAPI − JavaScript
  • aragonAPI − React
  • aragonAPI − Wrappers
  • aragonAPI − …

Or:

  • aragonAPI for JavaScript
  • aragonAPI for React
  • aragonAPI for wrappers
  • aragonAPI for …

Or we could put it after, but it feels a bit weird IMO:

  • JavaScript − aragonAPI
  • React − aragonAPI
  • Wrappers − aragonAPI
  • … − aragonAPI

This comment has been minimized.

Copy link
@sohkai

sohkai Apr 2, 2019

Member
aragonAPI for JavaScript
aragonAPI for React
aragonAPI for wrappers
aragonAPI for …

Sounds quite nice 👍

This comment has been minimized.

Copy link
@0xGabi

0xGabi Apr 2, 2019

Author Member

I also like aragonAPI for ...

I'm a bit confuse still about the extent of the JavaScript implementation. I thought all aragon.js was the JavaScript implementation of aragonAPI.

This comment has been minimized.

Copy link
@2color

2color Apr 3, 2019

Contributor

I understand the confusion. aragon.js is just the monorepo name. The suggestions above indicate the actual semantic meaning of each part of it.

This comment has been minimized.

Copy link
@2color

2color Apr 3, 2019

Contributor

This discussion got me thinking. Now when I think of @aragon/wrapper it's actually just the Client/Wrapper API in the sense that it provides an API for clients, e.g. CLI or the official client (aragon/aragon).

Not relevant directly to the PR though.

This comment has been minimized.

Copy link
@2color

2color Apr 3, 2019

Contributor

Following a conversation with @0xGabi I think it's important we indicate three things:

  • It's the App API
  • It's part of the Javascript implementation of aragonAPI

Given the other page with the explanation about the JavaScript explanation I think aragonAPI for Apps should be fine.

@@ -1,6 +1,6 @@
# aragonAPI for React
# aragonJS - React Hooks

This comment has been minimized.

Copy link
@sohkai

sohkai Apr 2, 2019

Member
Suggested change
# aragonJS - React Hooks
# JavaScript - React API

I think using "Hooks" directly is a bit misleading, since this package is meant to be used by all React devs, not just those who want hooks.

@0xGabi 0xGabi requested review from 0x6431346e, sohkai and bpierre and removed request for sohkai Apr 2, 2019

@0xGabi
Copy link
Member Author

0xGabi left a comment

I made the changes after review and included a file about Aragon app architecture to close aragon/hack#23

docs/APP.md Outdated
@@ -1,4 +1,4 @@
# App
# aragonAPI for Javascript

This comment has been minimized.

Copy link
@0xGabi

0xGabi Apr 2, 2019

Author Member

Not sure about this name, still a bit confuse about the extent of Javascript implementation of aragonAPI.

@0xGabi 0xGabi changed the title Docs update: include react hooks & fixes Docs update: include react api, aragon app architecture & fixes Apr 2, 2019

@2color 2color self-requested a review Apr 3, 2019

@@ -1,4 +1,4 @@
# Providers
# aragonAPI for providers

This comment has been minimized.

Copy link
@2color

2color Apr 3, 2019

Contributor

These are just providers.

@0xGabi 0xGabi merged commit cfb065e into aragon:master Apr 3, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
license/cla Contributor License Agreement is signed.
Details

Aragon Mesh Team automation moved this from Review to Done Apr 3, 2019

@0xGabi 0xGabi deleted the 0xGabi:docs/update-api branch Apr 3, 2019

@@ -4,9 +4,9 @@ A JavaScript implementation of aragonAPI, used to interact with aragonOS by hand

### Docs

- [React API](/packages/aragon-api-react/README.md)

This comment has been minimized.

Copy link
@sohkai

sohkai Apr 3, 2019

Member

Similar to the other comment, I'd make this a subcategory of "App API"

@@ -1,4 +1,4 @@
# aragonAPI for wrappers
# aragonAPI for Wrappers

This comment has been minimized.

Copy link
@sohkai

sohkai Apr 3, 2019

Member

Maybe we could change to say "API for Aragon client implementations"? Nobody really knows what the "wrapper" really is 😄

This comment has been minimized.

Copy link
@0xGabi

0xGabi Apr 3, 2019

Author Member

Yes, I think this is more clear

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.