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

Adding authorisation options for API access #1042

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bpiraeus
Copy link

Fixes #

Requirements

All new code should be covered with tests, documentation should be updated. CI should pass.

Description of the Change

Checklist

  • unit-test added (if change is algorithm)
  • functional test added/updated (if change is functional)
  • man page updated (if applicable)
  • bash completion updated (if applicable)
  • documentation updated
  • author name in AUTHORS

  - ldap currently the only supported method
adding authorisation options for local repositories
  - ldap groups per repo
@bpiraeus
Copy link
Author

I went through the failing tests, and unless I'm missing something, those are not from my changes, not sure what I can do about that short of refactoring someone else's code. I'm happy to muck around if needed, but you'd likely prefer someone with a lot more chops than myself to mitigate those issues.

@randombenj
Copy link
Member

@bpiraeus It looks like mostly deprecation errors

If you want to, feel free to investigate if there are some simple fixes for the deprecated packages (x/term) seems to be quite an easy one, not sure about the crypto one ...

@bpiraeus
Copy link
Author

Cleaned up a few oddballs in my code that gosimple flagged, also moved the terminal bits to the appropriate package, the pgp ones appear as if they're likely going to want 3rd party library.

@randombenj
Copy link
Member

@bpiraeus I just re reviewd the changes in go.mod and it seems like you upgraded quite a lot of dependencies. Was this on purpose? I think if you just add the packages you need to add without upgrading the others it will 'fix' the linting.

I would propose to upgrade all the other deps in a sepparate change. What do you think?

@neolynx neolynx self-assigned this Feb 6, 2024
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

3 participants