amplitudejs icon indicating copy to clipboard operation
amplitudejs copied to clipboard

Consistent json notation in documentation

Open rowild opened this issue 4 years ago • 11 comments

Why should this feature be implemented?

Personally, I find it a bit confusing that the json notation throughout the documentation is varying so much. There are places, where the keys have not quoted, other places show single and double quotes in the same example.

Maybe there is a chance to make this consistent? (I would love to prepare PRs, but I didn't find a place for the documentation on github... have I overseen something?)

Wonderful product! Congrats to the creators - you must be artists!!!

rowild avatar Nov 07 '19 11:11 rowild

I think this is a great idea. Docs can be found in the /docs folder: https://github.com/521dimensions/amplitudejs/tree/master/docs

@danpastori Do you have a preference what they should be?

Wonderful product! Congrats to the creators - you must be artists!!!

We appreciate the kind words 😃

jaydrogers avatar Nov 07 '19 13:11 jaydrogers

I also would like to mention, that the "tone" of the documentation is incredibly well chosen: very friendly, at the same time very encouraging. Goes quite a bit beyond what a documentation usually is and should definitely be a "show-off" example if a "How to write documentation" documentation.

And as for my taste: I'd go for a json notation with as little quotes as possible.

Thank you so much! Amplitude is so much fun!

rowild avatar Nov 07 '19 14:11 rowild

Hey @rowild, I spoke to @danpastori on this and he said the notation "depends".

Do you have two examples where they are different? Maybe from there if we do need to standardize it you could help us with a PR?

jaydrogers avatar Nov 22 '19 18:11 jaydrogers

Hey @jaydrogers ! Of course! It was on my agenda for next week anyway.

The main "issue" I noticed is the inconsequent JSON object notation for the init() example. In "Initialization" it is written with "songs" in quotes:

Amplitude.init({
        "songs": [
		{
			"name": "Song Name 1",
			"artist": "Artist Name",
			"album": "Album Name",
			"url": "/song/url.mp3",
			"cover_art_url": "/cover/art/url.jpg"
		}, ...
	]
});

white in "Soundcloud Configuration" "songs" as well as other nested keys (like "url") do not have quotes (and the values here use single quotes, while other examples use double quotes):

Amplitude.init({
    songs: [{
      url: 'https://soundcloud.com/some/url/to/some/song'
    }],
    soundcloud_client: 'YOUR_CLIENT_KEY'
  });

There are some PRs that remove quotes or change them from single to double quotes. Maybe the single quotes would be better, though.

I wait what you guys say. If you find this useless, please feel free to drop this issue - since it is really not that important. If you on the other hand think that this could improve the quality of the documentation, let me know and I'd be most happy to provide the changes in the format you wish.

rowild avatar Nov 22 '19 21:11 rowild

Thanks so much @rowild! We greatly appreciate your efforts.

I want to let you in on some transparency to the situation too... @danpastori and I are running a two person company called 521 Dimensions. Dan does all development and I do DevOps and UX design (I have zero JavaScript/CSS experience).

I understand JSON, but I definitely do not refer to myself as a developer. So I leave the "standardization" calls to Dan.

Since we are only a two person company, Dan is super busy with our commercial work so that we can keep the lights on.

He has seen all of your emails, he is very thankful for all of your work, but I am not sure when he will be able to go through everything.

Hopefully he gets some free time soon!

jaydrogers avatar Nov 22 '19 21:11 jaydrogers

@jaydrogers Thank you for your quick response! PLEASE let this not be anything that robs your time! It's not even peanuts!

I close this issue in favor of development!!!

My best wishes and my admiration to both of you!

rowild avatar Nov 22 '19 22:11 rowild

No problem. I am actually going to re-open this so that we can track it.

We're all for making everything that we do better. We just don't want to disrespect your time either!

jaydrogers avatar Nov 22 '19 22:11 jaydrogers

All right! :-) Never felt any kind of disrespect here! Quite the opposite! Just let me know, when I can be of help of some kind, whenever you find the time!

rowild avatar Nov 22 '19 22:11 rowild

Thanks, we greatly appreciate your kind words! You're already helping us out by submitting PRs. If you have any interest in any other capacity (you've already done plenty), here's how to help: https://521dimensions.com/open-source/amplitudejs/docs/contributing/

Thanks again!

jaydrogers avatar Nov 22 '19 22:11 jaydrogers

@jaydrogers I just learned that it "real JSON" requires to have double quotes around "property names" (or "keys"), for the sake of simplicity single quotes are forbidden. Otherwise a syntax error will be thrown. An old reason for that was to avoid problems with reserved names keywords in Javascript (like "function", "if", ...) in ECMA 3 times. ECMA 5 solved that problem, but since the intention of JSON is to a simple universal exchange format, double quotes are still required. If double quotes are missing, it is, technically, not JSON anymore, or at least "relaxed JSON" (which also allows comments). I wanted to add this here, since my ticket might lead into the wrong direction. In the sense of "universalness" double quotes should be put back to the examples, since it quite well might be the case, that the JSON is coming from a different source. Let me know what you think, and if you guys agree, I'd prepare a PR to reverse my wrong-doing.

rowild avatar Dec 01 '19 21:12 rowild

No problem! Thanks for the additional insight. I will leave the final call to @danpastori. He knows a lot more about this than me :-)

jaydrogers avatar Dec 02 '19 14:12 jaydrogers