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.

Sign up for GitHub

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

Jump to bottom

Fix revive lint warnings #556

Merged
merged 2 commits into from
Apr 22, 2022
Merged

Fix revive lint warnings #556

merged 2 commits into from
Apr 22, 2022

Conversation

nbarbey
Copy link
Contributor

@nbarbey nbarbey commented Apr 22, 2022

This is related to issue #36

All issues involve renaming function to drop a prefix corresponding to the package name.
The fix has been done automatically with a refactoring tool.

This creates a change in the public API as namespace.NamespaceWithComment is renamed to namespace.WithComment.

@nbarbey nbarbey requested a review from a team as a code owner April 22, 2022 14:57
@github-actions github-actions bot added area/schema Affects the Schema Language area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Apr 22, 2022
@github-actions
Copy link

github-actions bot commented Apr 22, 2022 •
edited

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

@nbarbey
Copy link
Contributor Author

nbarbey commented Apr 22, 2022

Hello folks ! Trying to contribute something simple here.
This creates a breaking change in the public API.
Let me know if you want to handle this differently.
Maybe having two functions and mark one as deprecated until next version ?

@nbarbey
Copy link
Contributor Author

nbarbey commented Apr 22, 2022

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

@nbarbey
Copy link
Contributor Author

nbarbey commented Apr 22, 2022

recheck

@nbarbey
Copy link
Contributor Author

nbarbey commented Apr 22, 2022

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

@nbarbey
Copy link
Contributor Author

nbarbey commented Apr 22, 2022

recheck

@nbarbey
Copy link
Contributor Author

nbarbey commented Apr 22, 2022

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

@nbarbey
Copy link
Contributor Author

nbarbey commented Apr 22, 2022

recheck

1 similar comment
@jzelinskie
Copy link
Member

recheck

@jzelinskie
Copy link
Member

Oh, I think your commits are lacking an email.

@nbarbey
Copy link
Contributor Author

nbarbey commented Apr 22, 2022

Ok got, thanks for your comment, it led me to the solution !
I authored the commits with my work email by mistake.
Sorry about this.

josephschorr
josephschorr approved these changes Apr 22, 2022
View reviewed changes
Copy link
Member

@josephschorr josephschorr left a comment

Choose a reason for hiding this comment

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

LGTM

@nbarbey @josephschorr
Fix revive lint warning in internal/namespace
36b8d6e 
Signed-off-by: Nicolas Barbey <nicolas.a.barbey@gmail.com>
@nbarbey @josephschorr
Fix revive lint warning in pkg/namespace
1ebe8b7 
Signed-off-by: Nicolas Barbey <nicolas.a.barbey@gmail.com>
@josephschorr josephschorr merged commit 53c5f77 into authzed:main Apr 22, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@josephschorr josephschorr josephschorr approved these changes

Assignees
No one assigned
Labels
area/schema Affects the Schema Language area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@nbarbey @jzelinskie @josephschorr