Skip to content
This repository has been archived by the owner on Mar 3, 2022. It is now read-only.

馃摎 Documentation/code quality improvements #993

Closed
balazsorban44 opened this issue Nov 22, 2019 · 8 comments
Closed

馃摎 Documentation/code quality improvements #993

balazsorban44 opened this issue Nov 22, 2019 · 8 comments

Comments

@balazsorban44
Copy link

13 of the 91 open issues mentions the word documentation or docs directly, maybe even more could have been resolved with better documentation. Looks like providing a high quality documentation would benefit not only the users of this package, but the maintainers as well.

I recommend beginning with the API, explaining the function signatures, comment those functions, and all parameters, not just "any" all types in the index.d.ts. Finding out how logger works was painful (there is not a word that you should set Logger to console).

I am a fan of declaration files, it makes it possible to contribute to the library even to those who are not familiar with TypeScript, but still get a good IDE experience. DefinitelyTyped may be interesting. After a while, maybe the community could take that over, so the maintainers have more time on the core stuff.

Secondly, updating the examples/samples would benefit many as well. Make them available online, so one does not have to clone the repo just to run them! A bit of coat of paint would not hurt those samples as well, but I understand it is not a priority, still could improve the quality of this repo.

Haven't looked at how bundling is done right now, but I am sure we should have some kind of code splitting, cause this package is HUGE. I have to load it async on the front-end, cause it takes up almost HALF of the entire bundle. Parcel 馃摝 sounds like a modern alternative.

@balazsorban44
Copy link
Author

balazsorban44 commented Dec 5, 2019

ping @brockallen, as you seem to be the main contributor/author

@brockallen
Copy link
Member

Great ideas, but I work on this project either in my free/spare time, or specifically for my customers. If the greater community gets benefit, then great, but that's not my primary audience. If I get time I will consider these suggestions. If you want to build your own docs project on oidc-client-js, then go for it.

@balazsorban44
Copy link
Author

I understand, thanks for the clarification. I thought this was some kind of official package meant for the masses, since it is listed in the Certified OpenID Connect Implementations (though the last mentioned version is oidc-client 1.3, so it must be a very outdated site!?).

I would gladly help with the docs, the main problem is that first I would need one, to understand the package better. 馃槄, also I am not the one who knows the ins and outs of OpenID. 馃槩

If it is only your "hobby project", it is understandable you have other priorities. Maybe this could/should be mentioned in the README to avoid future confusion then?

It also means, that my team and I maybe should look for an alternative, as we really do not want to focus on diving deep into source codes we are barely familiar with "just to get some basic auth working", it is just not our area of expertise. Do you know any other alternatives that may provide probably less features than this package, but is considerably smaller in size and is well-documented/meant as a true open source project?
Found Auth0 libraries better suited, but unfortunatelly not everything works with out custom IdP server.

@brockallen
Copy link
Member

Found Auth0 libraries better suited, but unfortunatelly not everything works with out custom IdP server.

Most product libraries are product specific, not protocol specific.

As for your comments, You misunderstand -- this is not just a hobby project. This project is meant for me and my customers. It's open source, so if you get benefit, then great. I will take your suggestions into consideration, so thanks for those. But since you're not a customer I don't foresee spending even more time to help you and your company, much in the same way you'd not give me your product for free.

I'll leave one last thought, which is nicely summarized here:
https://hueniverse.com/no-cure-for-entitlement-38eb3c80d808

@balazsorban44
Copy link
Author

balazsorban44 commented Dec 5, 2019

No need to call it a

parasitic demand

馃檪, neither I feel entitled to do anything, I am a junior developer who likes the idea of open source, and simply wanted to mention my findings.

But you said yourself these are great ideas! Also, as mentioned, OpenID recommends this package and calls it a "certified implementation". It very well be the case that I misinterpret it (in that case I apologize), but for me it looked like it is the "recommended" way for the community to use. If you have no involvment with OpenID, I do not understand the endorsment of this package there.

The package is also under a verified organization of IndentityModel, which again (for me) indicates there is big support from an organization behind it.

All I have done is I went through the issues, and interpreted that a huge number of them may be solveable by better documentation, which in the long run may result in significantly less issues, ie. more time fore actual development. I did not demand anything in my opinion. It is at the end of the day a public package with 73.000 weekly downloads according to npm. My assumptions were wrong, and I apologize, again.

Closing this issue, before any more misunderstanding should rise.

@brockallen
Copy link
Member

The issue is that the work involved is not trivial, and it's easy to come along and throw around tasks I should be doing. Perhaps that wasn't your intent, so no worries.

@leastprivilege
Copy link

The package is also under a verified organization of IndentityModel, which again (for me) indicates there is big support from an organization behind it.

I think verified means, that the email address is verified ;)

Like probably most developers, working on docs is our least enjoyable task - it is also really hard to write good documentation. If you think you can really improve the docs and thus reduce the number of issues, please go ahead and work on this! This would be very appreciated.

But in our experience, most people who thought in the past they can take on this task, simply stopped working on it halfway through - which often makes things even worse...but maybe you are different.

And that's also why we are more and more reluctant to just say "of course - we accept pull requests"...

@balazsorban44
Copy link
Author

Circulating back on this, as (in my opinion) I have now come to a better understanding of how OSS works...

I would like to again apologize @brockallen and @leastprivilege if I caused discomfort. This package in a way or another set me on a path that ultimately has led me to another project revolving around OIDC and OAuth over at https://github.com/nextauthjs/next-auth, which I am now a core maintainer of.

I have come to the understanding of how hard it is to maintain OSS in a way that everyone gets happy. (Spoiler, it is not possible to make everyone happy, and there always be people who don't see the faces/humans behind a project, but a good attitude on both sides can lead to a thriving community.)

Also, writing good (enough) documentation is hard! AuthN/AuthZ is also a topic that not many take interest in and just expect a solution that "just works", tailored for their needs. Reading through specs much more I can see a lot of ambiguity, so it is also easy to interpret things differently, which does not make things easier either.

Anyway, I just wanted to share this information, and I hope that you are doing great! Thank you for this library! 馃檹
馃檪

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants