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

enh: add documentation for snap-in build config #41

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

navneel99
Copy link
Contributor

Adding documentation for snap-in's build config.

Work-item

https://app.devrev.ai/devrev/works/ISS-84458

@navneel99 navneel99 requested a review from a team as a code owner April 26, 2024 17:06
Copy link
Collaborator

@bc-devrev bc-devrev left a comment

Choose a reason for hiding this comment

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

Need to add the new page to the sidebar config. @starlightknown can help with that.

fern/docs/pages/references/build_config.mdx Outdated Show resolved Hide resolved
fern/docs/pages/references/build_config.mdx Outdated Show resolved Hide resolved
fern/docs/pages/references/build_config.mdx Outdated Show resolved Hide resolved
Comment on lines 753 to 754
Build configuration is used to pass values during the snap-in build process. The values are present as environment variables. This is useful if the snap-in requires libraries that
are either private or from different npm registry than the default.

Choose a reason for hiding this comment

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

Build configuration is used to pass values .... - The "pass values" phrase doesn't seem correct. Should we instead rephrase it as Build configuration is used to configure the build process of the Snap-In. For e.g. It can be used to set the environment variables used during the build process. This is useful if ....

`type` can either be `constant` or `keyring`.

`value` is either the actual value of the environment variable or name of the developer keyring that
contains the secret depending on the type being `constant` or `keyring`.

Choose a reason for hiding this comment

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

Should we also link to the documentation page for build_configuration?

Comment on lines 1 to 3
Build config is a way to provide data to the snap-in build process. Currently, the only way to send data
is through environment variables that the build process has access to.

Choose a reason for hiding this comment

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

Ditto. See my suggestion below.

@codeon
Copy link
Contributor

codeon commented Apr 30, 2024

Are we ready to release the feature to the public?

Copy link
Contributor

@starlightknown starlightknown left a comment

Choose a reason for hiding this comment

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

LG just one unclear sentence

```

Since "AUTH_TOKEN" is sensitive information that should not be hardcoded in the manifest,
the developer can instead mention the name of the developer keyring and during snap-in version creation
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
the developer can instead mention the name of the developer keyring and during snap-in version creation
the developer can instead mention the name of the developer keyring and during snap-in version creation

and during?

@@ -0,0 +1,61 @@
The build configuration is used to configure the build process of the snap-in. For example, it can be used to set the environment variables used during the build process.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The definition is tautological: build configuration configures builds. The intro paragraph should have comprehensive statements, not an example. What else does it do besides set environment variables? I don't see anything else covered in this doc.

fern/docs/pages/references/build-config.mdx Outdated Show resolved Hide resolved
fern/docs/pages/references/manifest.mdx Outdated Show resolved Hide resolved
@bc-devrev
Copy link
Collaborator

@navneel99 is this PR still relevant? Please either take it forward or close it.

@navneel99
Copy link
Contributor Author

Hello, we currently don't want to make this public to 3P, but want to keep it internal to DevRev

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

6 participants