Link to wiki from Readme. #152

navyasric · 2018-05-24T21:15:49Z

Remove duplication. Moved removed content into wiki.

navyasric · 2018-05-24T21:17:06Z

@abhidnya13 Can you please add the badges for CI build, Reference docs. Thanks!

abhidnya13 · 2018-05-24T23:18:45Z

I have added the badges. The Travis CI build badge is of dev branch. I am not sure if it should be of dev or master. Let me know and I'll make those changes if needed.

rayluo

Thanks for taking the initiative to come up with this PR! The comments below is open for discussion. I'm just marking down some keypoints here, before I forget. We can always follow up this, either online or offline.

rayluo · 2018-05-25T01:07:46Z

README.md

@@ -1,72 +1,41 @@
 # Microsoft Azure Active Directory Authentication Library (ADAL) for Python
+=====================================


The markdown format here isn't exactly right.

A generic tips: you can visualize the markdown locally with some plugins in your browser. Or if you already commit and push, you can visualize it right from github.

Thanks for catching that.

rayluo · 2018-05-25T01:13:39Z

README.md


-  Upgrade to the latest pip (8.1.2 as of June 2016) and just do `pip install adal`.
+## Versions
+Current version - 0.6.0


I would suggest to not mention "current version" here, especially when we are already going to provide the link to the Release history page below. Customers can do a one-click and know the single-source-of-truth about the latest (and, all) version information.

The downside of mentioning it here is it introduces one more place for us to update during future releases. If we could avoid that, then we probably should. :-)

I was trying to match other SDKs but I agree with your suggestion of avoiding the duplication.

rayluo · 2018-05-25T01:26:16Z

README.md


-* For Linux
+Minimum recommended version - 0.6.0


Again, we probably won't need to hardcode yet another version number here.

On a side note, the idea of "minimum recommended version" is intriguing. What do we want to convey here?

If the intention is simply want to recommend customers to "always use whatever latest version is available" (which makes perfect sense), we can just write something like that, without hardcoding a specific version number.

By definition of Semantic Versioning, customers who using an old version X.Y.Z can always safely upgrade to newer version "X.Y-latest.Z-latest", without worrying about breaking change.

Or customers can technically stay with a version "X.Y.Z", as long as it still works.

Yes I think specifying that we follow semantic versioning and pointing to the Release page is sufficient.

Nitpicking: since we explicitly mention Semantic Versioning in our README, how about we add a link to its official website too?

rayluo · 2018-05-25T01:36:12Z

README.md


-### Install


I understand the motivation of "Remove duplication. Move content into wiki."
But do we consider keeping some most-seeking contents, such as "how to install", here in this de-facto project home page? In fact, we fine tuned those installation instructions a couple times before, based on real-world supporting request. Better not lose those valuable knowledge and efforts.

We are not losing the instructions recorded over time. They are just in the Wiki page now. I will link to it from the Readme here since it is a "most-seeked" content.

Oh I see. Last time I checked, I tried find-by-eye and also "CTRL+F" (i.e. find in page) but could not find keyword "install" in our Wiki home page, its sidebar, or the FAQ page. I was not aware that "install" is inside the "Basic".

Now you have kept this "most-seeked" content by a link to that very wiki page. That is a reasonable solution! 👍

rayluo · 2018-05-25T01:45:49Z

README.md


-  You'll need a C compiler, libffi + its development headers, and openssl + its development headers.
-  Refer to [cryptography installation](https://cryptography.io/en/latest/installation/)
+> Note: Changes on 'client_id' and 'resource' arguments after 0.1.0


Historically, these part showed up early in this document; and then in one commit we purposely move it to the bottom of this document, and the reasoning was documented in that commit.

@rayluo Thanks for the context. I placed it under versions since it is information related to a specific version and it is a grey text note to draw little attention. But if this is mostly historical content, is it still relevant and should we remove it instead?

@navyasric You raised a very good point! At a hindsight, those paragraph "Changes ... after 0.1.0" really should belong to its release note. I've just updated the release note on version 0.3.0, you can go ahead to delete this paragraph from README now. Thank you!

rayluo · 2018-05-25T01:49:30Z

README.md

-  If you still like to build from source, refer to [cryptography installation](https://cryptography.io/en/latest/installation/).
-  For more context, start with this [stackoverflow thread](http://stackoverflow.com/questions/22073516/failed-to-install-python-cryptography-package-with-pip-and-setup-py).
-
-### Acquire Token with Client Credentials


Although we do have and mention the "sample applications on GitHub" later in this README, but those samples locating in a separated repo may or may not cover all the scenarios ADAL Python supports (as of today, they don't). It would seem still beneficial to keep this minimal "out-of-box" samples sections here. What do you think?

This content was moved to this Wiki page which links to relavant dev sample for each flow. I will link to this page from Readme.

Historically we had a set of real code snippet right here in the README, later we changed it into the current form as a link to those real samples, after we were called out that they became out-of-sync. See full details here.

Now, moving these content to wiki page by actually duplicating those code snippets there, would bring us back to the potential 2-copies-would-become-out-of-sync situation. In fact, as a real-world example, one of our current wiki page technically already outdated, it includes an api_version=None which is no longer needed since our latest 1.0.0 release.

So my suggestion is, because anything that can go wrong will go wrong, so a better approach is to not create duplicate which we would forget to update later.

UPDATE:

We now updated the wiki page to contain only links back to the real code in current repo.

We could keep the same set of links here in README here (because the README and real code are in same repo therefore more likely to be in-sync), but instead we chose to just point to the wiki from README, so that we maintain only one set of links.

rayluo · 2018-05-25T01:56:54Z

README.md

-``` $ pip install adal ```
-
-### http tracing/proxy
-If you need to bypass self-signed certificates, turn on the environment variable of `ADAL_PYTHON_SSL_NO_VERIFY`


AFAIK, this particular line was the only place we document this behavior. If we are going to remove it here, do we plan to document it somewhere in the wiki then?

PS: We did mention it recently in API reference doc. But I assume DevOps will not necessarily read API Reference doc. I could be wrong, though.

This content is moved to the Basics page of the Wiki and I'll be linking to this page under installation part of Readme.

Bumping version number

Deleting extra lines

ADAL Python 0.7.0

Release 1.0.0 with changed behavior

Remove duplication. Moved removed content into wiki.

…rectory-library-for-python into nc-readme-updates

navyasric · 2018-05-31T21:18:56Z

@rayluo I have made some updates based on your feedback. Please take a look and let me know. Thanks.

rayluo

Thanks for the update! Added some more thoughts below.

rayluo · 2018-05-31T22:57:39Z

README.md


-It is recommended to read the [Auth Scenarios](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-authentication-scenarios) doc, specifically the [Scenarios section](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-authentication-scenarios#application-types-and-scenarios).  For some topics about registering/integrating an app, checkout [this doc](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-integrating-applications).  And finally, we have a great topic on the Auth protocols you would be using and how they play with Azure AD in [this doc](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-protocols-openid-connect-code).
+This library follows Semantic Versioning.


Since we are mentioning this guiding principle anyway, it would be convenient to also include a link to point to its official website.

rayluo · 2018-05-31T22:59:09Z

README.md


-While Python-specific samples will be added into the aforementioned documents as an on-going effort, you can always find [most relevant samples just inside this library repo](https://github.com/AzureAD/azure-activedirectory-library-for-python/tree/dev/sample).
+> Note: Changes on 'client_id' and 'resource' arguments after 0.1.0


Thanks for your suggestion. We now retrofit that old release note. We can go ahead to delete this section here now.

rayluo · 2018-05-31T23:17:21Z

README.md

-  If you still like to build from source, refer to [cryptography installation](https://cryptography.io/en/latest/installation/).
-  For more context, start with this [stackoverflow thread](http://stackoverflow.com/questions/22073516/failed-to-install-python-cryptography-package-with-pip-and-setup-py).
-
-### Acquire Token with Client Credentials


Historically we had a set of real code snippet right here in the README, later we changed it into the current form as a link to those real samples, after we were called out that they became out-of-sync. See full details here.

Now, moving these content to wiki page by actually duplicating those code snippets there, would bring us back to the potential 2-copies-would-become-out-of-sync situation. In fact, as a real-world example, one of our current wiki page technically already outdated, it includes an api_version=None which is no longer needed since our latest 1.0.0 release.

So my suggestion is, because anything that can go wrong will go wrong, so a better approach is to not create duplicate which we would forget to update later.

UPDATE:

We now updated the wiki page to contain only links back to the real code in current repo.

We could keep the same set of links here in README here (because the README and real code are in same repo therefore more likely to be in-sync), but instead we chose to just point to the wiki from README, so that we maintain only one set of links.

navyasric · 2018-06-01T21:04:03Z

@rayluo I have incorporated your feedback to keep the wiki mostly conceptual. I have modified the acquire token section of the wiki to match the format where we link to the code snippet in samples and minimize duplication. (Where there was no sample, I link to method signature).
This page is linked from the Readme under samples as well as the Basics page in wiki so that we have one copy of this document. Please let me know if this PR is good to merge now.

rayluo

Thanks @navyasric for all the hard work! Merging now.

Link to wiki from Readme.

5377055

Remove duplication. Moved removed content into wiki.

navyasric requested review from rayluo and abhidnya13 May 24, 2018 21:15

Adding badges for Travis CI dev branch and read the docs

a2a7351

rayluo reviewed May 25, 2018

View reviewed changes

abhidnya13 and others added 11 commits May 29, 2018 13:12

ADAL Python 0.7.0

3c59011

Bumping version number

Update token_request.py

2e4d18a

Deleting extra lines

Merge pull request #154 from AzureAD/release-0.7.0

c076319

ADAL Python 0.7.0

Release 1.0.0 with changed behavior

2f39a09

Adding build badge for master too

6d8e8d4

Merge pull request #155 from AzureAD/release-1.0.0

aab7fbe

Release 1.0.0 with changed behavior

Link to wiki from Readme.

1a40798

Remove duplication. Moved removed content into wiki.

Adding badges for Travis CI dev branch and read the docs

39e37f4

Adding build badge for master too

23ccb27

Merge branch 'nc-readme-updates' of github.com:AzureAD/azure-activedi…

64b88f5

…rectory-library-for-python into nc-readme-updates

Add link to relavant wiki pages

fa07dbf

rayluo reviewed May 31, 2018

View reviewed changes

Add link to sematic version

bbc9c4b

rayluo approved these changes Jun 4, 2018

View reviewed changes

rayluo merged commit d2e0878 into dev Jun 4, 2018

rayluo deleted the nc-readme-updates branch June 11, 2018 17:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Link to wiki from Readme. #152

Link to wiki from Readme. #152

navyasric commented May 24, 2018

navyasric commented May 24, 2018

abhidnya13 commented May 24, 2018

rayluo left a comment

rayluo May 25, 2018

navyasric May 31, 2018

rayluo May 25, 2018

navyasric May 31, 2018

rayluo May 25, 2018

navyasric May 31, 2018

rayluo May 31, 2018

rayluo May 25, 2018 •

edited

Loading

navyasric May 31, 2018

rayluo May 31, 2018

rayluo May 25, 2018

navyasric May 31, 2018

rayluo May 31, 2018

rayluo May 25, 2018

navyasric May 31, 2018

rayluo May 31, 2018 •

edited

Loading

rayluo May 25, 2018

navyasric May 31, 2018 •

edited

Loading

navyasric commented May 31, 2018

rayluo left a comment

rayluo May 31, 2018

rayluo May 31, 2018

rayluo May 31, 2018 •

edited

Loading

navyasric commented Jun 1, 2018

rayluo left a comment

		@@ -1,72 +1,41 @@
		# Microsoft Azure Active Directory Authentication Library (ADAL) for Python
		=====================================


		It is recommended to read the [Auth Scenarios](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-authentication-scenarios) doc, specifically the [Scenarios section](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-authentication-scenarios#application-types-and-scenarios). For some topics about registering/integrating an app, checkout [this doc](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-integrating-applications). And finally, we have a great topic on the Auth protocols you would be using and how they play with Azure AD in [this doc](https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-protocols-openid-connect-code).
		This library follows Semantic Versioning.


		While Python-specific samples will be added into the aforementioned documents as an on-going effort, you can always find [most relevant samples just inside this library repo](https://github.com/AzureAD/azure-activedirectory-library-for-python/tree/dev/sample).
		> Note: Changes on 'client_id' and 'resource' arguments after 0.1.0


		### Install

Link to wiki from Readme. #152

Link to wiki from Readme. #152

Conversation

navyasric commented May 24, 2018

navyasric commented May 24, 2018

abhidnya13 commented May 24, 2018

rayluo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rayluo May 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rayluo May 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

navyasric May 31, 2018 • edited Loading

Choose a reason for hiding this comment

navyasric commented May 31, 2018

rayluo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rayluo May 31, 2018 • edited Loading

Choose a reason for hiding this comment

navyasric commented Jun 1, 2018

rayluo left a comment

Choose a reason for hiding this comment

rayluo May 25, 2018 •

edited

Loading

rayluo May 31, 2018 •

edited

Loading

navyasric May 31, 2018 •

edited

Loading

rayluo May 31, 2018 •

edited

Loading