Skip to content

Unify RBAC flow in a single module #426

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

Merged
merged 1 commit into from
May 30, 2023
Merged

Conversation

nitisht
Copy link
Member

@nitisht nitisht commented May 30, 2023

This commit combines all the RBAC specific code in a single
crate. This improves readability.

Also added more relevant comments

@nitisht nitisht requested a review from trueleo May 30, 2023 08:34
@github-actions
Copy link
Contributor

github-actions bot commented May 30, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@nitisht
Copy link
Member Author

nitisht commented May 30, 2023

I have read the CLA Document and I hereby sign the CLA

@nitisht
Copy link
Member Author

nitisht commented May 30, 2023

recheck

github-actions bot added a commit that referenced this pull request May 30, 2023
@nitisht
Copy link
Member Author

nitisht commented May 30, 2023

@trueleo PTAL

@trueleo
Copy link
Contributor

trueleo commented May 30, 2023

@nitisht Looks all good to me but one small thing is that we should keep src/rbac.rs on top level instead of moving it mod.rs

from the book

src/front_of_house/hosting.rs (what we covered)_
src/front_of_house/hosting/mod.rs (older style, still supported path)

If you use both styles for the same module, you’ll get a compiler error. Using a mix of both styles for different modules in the same project is allowed, but might be confusing for people navigating your project.

The main downside to the style that uses files named mod.rs is that your project can end up with many files named mod.rs, which can get confusing when you have them open in your editor at the same time.

crate. This improves readability.

Also added more relevant comments
@nitisht
Copy link
Member Author

nitisht commented May 30, 2023

@nitisht Looks all good to me but one small thing is that we should keep src/rbac.rs on top level instead of moving it mod.rs

from the book

src/front_of_house/hosting.rs (what we covered)_
src/front_of_house/hosting/mod.rs (older style, still supported path)
If you use both styles for the same module, you’ll get a compiler error. Using a mix of both styles for different modules in the same project is allowed, but might be confusing for people navigating your project.
The main downside to the style that uses files named mod.rs is that your project can end up with many files named mod.rs, which can get confusing when you have them open in your editor at the same time.

I see..updated the PR accordingly

@trueleo trueleo changed the title Unify RBAC flow in a single crate Unify RBAC flow in a single module May 30, 2023
@nitisht nitisht merged commit ca1f427 into parseablehq:main May 30, 2023
@nitisht nitisht deleted the auth-cleanup branch May 30, 2023 10:18
@github-actions github-actions bot locked and limited conversation to collaborators May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants