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

Docs: Document fixing issues, project roots, and new features #1043

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

anderseknert
Copy link
Member

This is a follow-up PR to #1035, documenting the new capabilities of Regal added there

@anderseknert anderseknert marked this pull request as ready for review September 4, 2024 07:27
Copy link
Member

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

Reviewed above-the-fold, skimmed the rest

README.md Outdated
using the JSON representation of the Rego abstract syntax tree (AST) as input, a
few additional custom built-in functions and some indexed data structures to help
with linting.
- Deliver an outstanding policy development experience by providing the best possible tools for that purpose
Copy link
Member

Choose a reason for hiding this comment

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

Move to the top of Goals, perhaps? Nobody reads the last goal :P

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough :)

README.md Outdated
[OPA VS Code extension](https://marketplace.visualstudio.com/items?itemName=tsandall.opa)
- Get started with Regal for Zed by installing the [zed-rego](https://github.com/StyraInc/zed-rego) extension
- Get started with Regal in Neovim using any of the available
[Regal plug-ins](https://docs.styra.com/regal/editor-support#neovim)
Copy link
Member

Choose a reason for hiding this comment

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

main has a section on Helix. Worth adding?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's good to list all the editors here, but I'd list them on one line, rather than one each. This way it doesn't take up so much space, but people can still scan for their editor.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixing!

docs/fixing.md Outdated
regal fix <path> [path [...]] [flags]
```
:::tip
Before you make any automated fixes, make sure to commit any other changes you may have done in your project! That way
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Before you make any automated fixes, make sure to commit any other changes you may have done in your project! That way
Before you make any automated fixes, make sure to commit or stash any other changes you may have done in your project! That way

Regal is a linter and language server for [Rego](https://www.openpolicyagent.org/docs/latest/policy-language/), helping
you write better policies and have fun while doing it!
Regal is a linter and language server for [Rego](https://www.openpolicyagent.org/docs/latest/policy-language/), making
your Rego magnificent, and you the ruler of rules!
Copy link
Member

Choose a reason for hiding this comment

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

:)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it went well with the "regal" theme 👑 😄

README.md Outdated
[OPA VS Code extension](https://marketplace.visualstudio.com/items?itemName=tsandall.opa)
- Get started with Regal for Zed by installing the [zed-rego](https://github.com/StyraInc/zed-rego) extension
- Get started with Regal in Neovim using any of the available
[Regal plug-ins](https://docs.styra.com/regal/editor-support#neovim)
Copy link
Member

Choose a reason for hiding this comment

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

I think it's good to list all the editors here, but I'd list them on one line, rather than one each. This way it doesn't take up so much space, but people can still scan for their editor.

README.md Outdated
By default, the answer to that question would be the **project**, or **workspace** root. But if the file was found
in a subdirectory containing a **bundle**, the directory naturally belongs under that *bundle's root* instead. The
`roots` configuration option under the top-level `project` object allows you to tell Regal where these roots are,
and have features that depend on this information work as expected.
Copy link
Member

Choose a reason for hiding this comment

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

Worth calling out which features these are?


The configuration file is not the only way Regal may determine project roots. Other ways include:

- A directory containing a `.manifest` file will automatically be registered as a root
Copy link
Member

Choose a reason for hiding this comment

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

This feels like it belongs int the By Default sentence above, or at least a reference to. I'd suggest this first over the manual config - or is that no longer preferred?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changing "By default" to "Normally", as I don't think using .manifest files is a norm.. even though we should help encourage that.

Honestly, I've not made up my mind yet on whether explicit config for roots or .manifest files is my preferene. I have also been pondering whether explicit configuration should override anything else. I think that perhaps it should... as if you explicitly tell us "use bundle1 as root", maybe we shouldn't go and "use bundle1 and bundle3" as roots 🤔

But I don't feel like this would be terrible to change later, so I'm good with what we got.

This is a follow-up PR to #1035, documenting the new capabilities
of Regal added there

Signed-off-by: Anders Eknert <anders@styra.com>
@anderseknert anderseknert merged commit 02d9538 into main Sep 4, 2024
4 checks passed
@anderseknert anderseknert deleted the dir-pkg-docs branch September 4, 2024 12:03
srenatus pushed a commit to srenatus/regal that referenced this pull request Oct 1, 2024
…nc#1043)

This is a follow-up PR to StyraInc#1035, documenting the new capabilities
of Regal added there

Signed-off-by: Anders Eknert <anders@styra.com>
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.

3 participants