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(schema): Add reasons to anti-examples, update README, rearrange individual schema layout #287

Merged
merged 8 commits into from
Nov 3, 2020

Conversation

colesimmons
Copy link
Contributor

Howdy! This PR:

  • Adds a new format for writing anti-examples that includes an explanation as to why they aren't valid (see Confusing examples and anti-examples #284)
  • Updates the README with the Yarn instructions (ran into some hiccups getting this set up after following the previous npm examples)
  • Removes pattern from the details section for schemas that do not have one (which is almost all of them), as having pattern: n/a immediately below, for e.g., type: object could imply that the object does not follow a particular pattern
  • Hyperlinks the text "Source Code" instead of displaying and hyperlinking from the path to the schema file, which made the schema documentation look more internal and added visual cruft
  • Moves examples and anti-examples below the properties table, as it makes more sense to show examples for something after you've defined it

It isn't quite ready for merge yet – I didn't have time to do a proofreading once-over, and I see that the formatting for anti-example explanations doesn't work correctly when they come after code blocks.

FWIW, here are some other suggestions that I think would help with the overall developer and/or contributor experience:

  • Add a default column to the properties tables
  • Combine identical schemas, like the action operation schemas
  • Have a sidebar on the GItHub Pages version of the documentation with the list of schemas. Overall pretty hard to navigate.
  • Use Mocha instead of Jest upon zapier init. Took me longer than I'd like to admit to figure out why the --grep option wasn't working (relatively new to JS testing frameworks)
  • Is additionalProperties needed for some reason in the schema files? It's always false.
  • Remove extra auth schemas that are just empty objects
  • Simplify /FunctionSchema
  • Order the keys in the properties tables, move required to top and experimental to bottom
  • Rephrase enums. Rather than string in ('lorem', 'ipsum'), maybe one of?
  • Consistent active/passive voice in properties descriptions. Some start with verbs (e.g. "Define how this create method.."), some start with nouns ("A function to customize how...")
  • Mention mutually exclusive keys in their descriptions. E.g. "Cannot be combined with children."
  • minLength and maxLength are defined on a lot of the schemas but never mentioned in the documentation.

Also the very first "zapier-platform-schema" hyperlink on this page leads to a 404.

Overall, I absolutely love Zapier and can't wait to see what our users at Abstract do with it. I did, however, in this process find a lot of the documentation wanting, especially for the CLI/schema when compared to the Visual Builder. Even in the Visual Builder docs, I noticed a few typos. My 2¢ is that it's worth investing more resources into, though I'm sure y'all have a ton on your plate(s) and are thinking "screw this guy" right about now 😅

Copy link
Contributor

@xavdid xavdid left a comment

Choose a reason for hiding this comment

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

@colesimmons thank you for this PR and the time you put into it! This is really spectacular.

I like the reordering in the markdown. Honestly we haven't spent much time on the schema docs since this product launched 3+ years ago, so it's great that they're getting some love. Believe me, I've been banging the documentation drum internally through my entire tenure here, but we've got a lot of ground to cover and it can be a hard project to sell/prioritize. But, one of our 2020 goals is "self service", so docs will almost certainly need to be a big part of that. Your suggestions to that end are super helpful and will definitely be incorporated into whatever planning happens there.

Re this site, that's a Github Pages site that corresponds to an archived repo - TIL they don't take those down! I'll see about getting it un-published.

Anyway, I looked through your changes and they look great! I didn't notice anything major that would stop us from merging this. I'll preemptively approve and you can let us know when you're ready for us to merge it!

Also, we'll be cutting a new CLI + Core + Schema version today or tomorrow, so you'll probably need to re-merge master after that happens so that your code links are correctly updated.

packages/schema/lib/schemas/FunctionSourceSchema.js Outdated Show resolved Hide resolved
@colesimmons
Copy link
Contributor Author

Thanks so much, David!

I definitely understand how it never feels like the right time to prioritize. It almost felt hypocritical to drop these comments when our own documentation has rarely been a priority. I always look forward to the holidays as a chance to clean up all the little things as the feature frenzy slows.

Overall, to provide a general theme to my comments above, the thing that would be the most helpful is making it all a more guided experience. I skimmed the lengthy intro and jumped straight into "Getting Started." It wasn't until I was in pretty deep already that I realized the CLI existed and was a much better option for our use case.

It seems like the documentation could start w/ the general documentation, where it explains why you should build, the expected time investment, the publishing process, the changelog, and most importantly, the difference between the different ways of building (this should be high up). From there, you could have separate documentation for the Visual Builder, the CLI, the Legacy Visual Builder, and the Partner program. A good example of what I have in mind here is the GCP App Engine documentation. You are shown the differences between the environments and have to select one before they show you any environment-specific documentation.

For the CLI, it'd be useful to get more opinionated. Even if you need to support multiple ways of defining an individual schema object internally, the multitude of options for many of them (i.e. a FunctionSchema) can be overwhelming and/or confusing. If I hadn't started in the Visual Builder, exported to the CLI, and then followed the existing patterns, I likely wouldn't have been able to figure it out.

I do also know well the difficulty in writing clear and concise documentation when you have tunnel vision, so I'd be more than happy to be an extra pair of eyes should you need them! My email is on my profile.

I pulled in the new version, fixed the formatting of anti-example reasons that follow code blocks, and added that missing parenthesis. FWIW AppSchema uses the version number in the examples, so if you have some sort of internal checklist on things to update when you cut a version, it'd be worth adding that.

Ready to merge!

@xavdid
Copy link
Contributor

xavdid commented Nov 3, 2020

That totally makes sense. I really appreciate such actionable feedback! I think you're totally write that some firmer guidelines would be helpful for new-to-Zapier-devs. When the platform is so powerful that it can do anything, it's hard to know where to start. You've given us some great things to mull over once we kick this off. I've set a note to circle back with you once we have something to show. 😁

In the meantime, I'll go ahead and merge this. Thanks again!

@xavdid xavdid merged commit f6cbd8c into zapier:master Nov 3, 2020
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.

None yet

2 participants