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

feat: implement createPersonalAccessToken #686

Open
wants to merge 1 commit into
base: 6.x
Choose a base branch
from

Conversation

maltem-za
Copy link
Contributor

Add support for Personal Access Token creation (#653)

renames ImpersonationToken > PersonalAccessToken and adds missing scopes, factors out common logic
@@ -9,12 +9,12 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonValue;

public class ImpersonationToken {
public class PersonalAccessToken {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Renaming the class like this is a breaking change for the API consumer.

@jmini
Copy link
Collaborator

jmini commented Apr 13, 2023

@maltem-za since the change is breaking, would it be OK to do in our next major version 6.0.0 on the 6.x branch?

@maltem-za
Copy link
Contributor Author

Hi @jmini, yes of course, whatever works for you. I switched to my own fork anyway as I needed this at the time that I opened the PR (it's >2 years old now). You're right of course that it's a breaking change, but in my mind it's a pretty simple refactor on the consumer side.

I stopped opening new PRs because it didn't seem likely they'd get merged, but you may be interested in integrating some of the other changes I've been making in my fork (mostly keeping tests working for later GitLab versions, and more recently I implemented OAuth2 access token refresh here). Happy to help where I can, time permitting :)

@jmini jmini changed the base branch from main to 6.x April 14, 2023 05:48
@jmini
Copy link
Collaborator

jmini commented Apr 14, 2023

@maltem-za:
I have rebased your change on top of origin/6.x but I am not able to force push it on your branch add-pats-support.

Probably you did not check [x] allowed edits from maintainers


As for working on a fork or in the upstream project, this is always a dilemma.
I joined this repo recently to not have to maintain my own fork, but I understand your reasons (and 2 years ago there was not much activity on the project).

@maltem-za
Copy link
Contributor Author

@jmini Hmm, I can't find that option. Do I need to open a new PR?

I'd be happy to contribute more directly again if things start moving a bit faster, and I would also prefer to not have to maintain a fork. Unfortunately I won't be able to make the switch to Java 11+ that quickly, but I guess the plan is to run 5.x and 6.x in parallel for some time anyway. I'll keep an eye on how things progress. Thanks for picking this up!

@jmini
Copy link
Collaborator

jmini commented Apr 22, 2023

I have opened a new PR for this #963.

On the main branch (version 5.x.x), my suggestion is to keep the old name ImpersonationToken and to do the renaming to PersonalAccessToken only with version 6.x.x.

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