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

Document private functions with @internal tag #2205

Merged

Conversation

craicoverflow
Copy link

@craicoverflow craicoverflow commented Sep 27, 2019

Closes #2183

Manually added an @internal JSDoc tag to exported functions that are not part of the public API.

I began by by making an ESLint rule which lives in the resources directory.

Although this helped me to identify the exported functions which are not exported in src/index.js, the rule is not yet stable enough to be included and I may look to follow this up with a separate PR, dealing exclusively with the ESLint rule.

@craicoverflow craicoverflow force-pushed the style/annotate-internal-functions branch from 74da857 to 27b8641 Compare September 27, 2019 15:26
@craicoverflow craicoverflow marked this pull request as ready for review September 27, 2019 15:39
@wtrocki
Copy link
Contributor

wtrocki commented Sep 27, 2019

I can confirm the same values locally. I just did not add internal flags to src/__tests__/starWarsData.js

@@ -122,6 +122,8 @@ function getCharacter(id) {

/**
* Allows us to query for a character's friends.
*
* @internal
Copy link
Member

Choose a reason for hiding this comment

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

@craicoverflow Thanks a lot for PR 👍
One I didn't mention is that __tests__ & __fitures__ are not part of NPM package so all function exported from these folders are private by definition.
So can you please revert all changes under __tests__ folder?

@@ -7,6 +7,8 @@ export type Path = {|

/**
* Given a Path and a key, return a new Path containing the new key.
*
* @internal
Copy link
Member

Choose a reason for hiding this comment

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

Same goes for jsutils folder:
https://github.com/graphql/graphql-js/blob/master/src/jsutils/README.md

These functions are not part of the module interface and are subject to change.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I can change that 👍

It's maybe worth noting that in this same file, pathToArray is exported as public in src/index.js (aliased as responsePathAsArray).

responsePathAsArray,

Copy link
Member

Choose a reason for hiding this comment

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

@craicoverflow Good catch 👍 I need to think about it 🤔 💭

@craicoverflow craicoverflow force-pushed the style/annotate-internal-functions branch from 20c91a7 to 1f835bb Compare September 30, 2019 07:43
@craicoverflow craicoverflow force-pushed the style/annotate-internal-functions branch from 1f835bb to 88b4aa8 Compare September 30, 2019 07:45
@IvanGoncharov IvanGoncharov added the PR: polish 💅 PR doesn't change public API or any observed behaviour label Oct 1, 2019
@IvanGoncharov IvanGoncharov changed the title style(JSDoc): document private functions with @internal tag Oct 1, 2019
@IvanGoncharov IvanGoncharov merged commit 5eb7c4d into graphql:master Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: polish 💅 PR doesn't change public API or any observed behaviour
3 participants