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

repo: Add README #13

Merged
merged 1 commit into from
Jul 24, 2024
Merged

repo: Add README #13

merged 1 commit into from
Jul 24, 2024

Conversation

minor-fixes
Copy link
Contributor

This change adds the beginning of a README, which covers basic topics such as:

  • how to build
  • how to report vulnerabilities
  • how to contribute

Tested: N/A

Bug: linear/CUS-360

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated
Comment on lines 3 to 4
This repository contains source for the authentication CLI that manages
credentials for EngFlow clusters.
Copy link
Contributor

Choose a reason for hiding this comment

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

Rephrase to emphasize the problem this solves for user:

Suggested change
This repository contains source for the authentication CLI that manages
credentials for EngFlow clusters.
This repository provides `engflow_auth`, a [Bazel credential helper](https://blog.engflow.com/2023/10/20/secure-builds-with-credential-helpers/) that helps you automatically obtain and securely store EngFlow authentication tokens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

README.md Show resolved Hide resolved
README.md Outdated

## Reporting Issues

To report bugs/vulnerabilities on `engflow_auth`, please send an email to
Copy link
Contributor

Choose a reason for hiding this comment

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

For normal bugs and feature requests, I think it's easiest to use GitHub issues.

For security vulnerability reports, [email protected] is a good idea. That should be used when the issue shouldn't be disclosed to the open internet until it's fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For normal bugs and feature requests, I think it's easiest to use GitHub issues.

We have issues disabled on this repo (I guess I was wrong about issues being coupled with pull requests!) and I think that's the right choice - issues invite a sense of urgency/SLA that we may not be ready for.

For features/bugs, I don't mind going through DSE's so that the rest of the org is aware, and priority is assigned correctly. Though - it may be beneficial to have some line of communication as a "fast path" for quicker turnaround, or to hear feedback from people who may not be customers

Copy link
Contributor

Choose a reason for hiding this comment

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

Either way is fine I think. We do have a well-established support channel for the platform, and we can use that for the CLI.

Mainly I think it's important to distinguish security vulnerability reports that need to be kept secret from everything else.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's important here to distinguish two separate channels. [email protected] is only for vulnerability reports that need to be handled discreetly. All other bugs and feature requests should go through DSE.

In our public documentation, we do have https://docs.engflow.com/support/index.html to explain our support channels (though it appears that page could use some clean up).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok - I think all the reports will end up in the non-urgent category, so I linked deeper to https://docs.engflow.com/support/get-day-to-day-support.howto.html - PTAL

README.md Show resolved Hide resolved
README.md Outdated

## Building

The CLI can be built via either the Golang toolchain or Bazel; released binaries
Copy link
Contributor

Choose a reason for hiding this comment

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

I apologize, I'm give the most pedantic of nits here.

Use "Go" as the name of the language. "Golang" is not an official name and shouldn't be used in a formal context like documentation. https://go.dev/doc/faq#go_or_golang

Suggested change
The CLI can be built via either the Golang toolchain or Bazel; released binaries
The CLI can be built via either the Go toolchain or Bazel; released binaries

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

README.md Show resolved Hide resolved
README.md Outdated
Comment on lines 28 to 29
Since we have no CLA set up for `engflow_auth`, we are not accepting pull
requests from external contributors at this time.
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
Since we have no CLA set up for `engflow_auth`, we are not accepting pull
requests from external contributors at this time.
We are not accepting pull requests from external contributors at this time.

Let's omit the rationale here. We could set up a CLA if we wanted to, but most changes here also require a change to a proprietary server-side component, so we want to decide and coordinate those changes ourselves.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made the rationale a bit more vague - no rationale sounds a bit standoffish to me, which I'd like to avoid

README.md Outdated
instance:

```
build:example --credential_helper=example.cluster.engflow.com=/path/to/engflow_auth
Copy link
Contributor

Choose a reason for hiding this comment

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

Use engflow as the config name rather than example, following https://docs.engflow.com/re/client/bazel-first-time.html#4-set-up-bazelrc. The user needs a bunch of other flags like --remote_executor, and we use engflow as the configuration name in those instructions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

README.md Outdated

### Use

1. Each day, run `engflow_auth login [CLUSTER URL]` each day to obtain an auth
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels a little awkward as a list with only one step. Maybe make "click the URL" a second step, or just format as a paragraph.

Also "each day" is here twice in the same sentence. Drop at least one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops! Fixed

README.md Outdated

## Reporting Issues

To report bugs/vulnerabilities on `engflow_auth`, please send an email to
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's important here to distinguish two separate channels. [email protected] is only for vulnerability reports that need to be handled discreetly. All other bugs and feature requests should go through DSE.

In our public documentation, we do have https://docs.engflow.com/support/index.html to explain our support channels (though it appears that page could use some clean up).

@jayconrod
Copy link
Contributor

Still needs resolution, looks good otherwise.

I think it's important here to distinguish two separate channels. [email protected] is only for vulnerability reports that need to be handled discreetly. All other bugs and feature requests should go through DSE.

In our public documentation, we do have https://docs.engflow.com/support/index.html to explain our support channels (though it appears that page could use some clean up).

This change adds the beginning of a README, which covers basic topics
such as:

* how to build
* how to report vulnerabilities
* how to contribute

Tested: N/A

Bug: linear/CUS-360
Copy link
Contributor

@jayconrod jayconrod left a comment

Choose a reason for hiding this comment

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

Looks good!

@minor-fixes minor-fixes merged commit 7cdd7a5 into main Jul 24, 2024
4 checks passed
@minor-fixes minor-fixes deleted the scott/CUS-360 branch July 24, 2024 00:11
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.

2 participants