Migrate from organisation keys to PATs

[RealOrangeOne]

Fixes #78

[tlimoncelli]

Is this a breaking change? Should the major version be bumped?

[RealOrangeOne]

Is this a breaking change? Should the major version be bumped?

Yes, and yes. I've not done any versioning changes here as those should be done as part of the release step.

[RealOrangeOne]

@nlewo (sorry for the ping) anything I can do to help progress this along? Keen to get PATs supported ASAP so it can be included in the terraform provider too.

[nlewo]

Hello @RealOrangeOne
Sorry for the delay, so thx for the ping!

I actually would prefer to keep the support of the API key even if it is deprecated, because this is still used by a lot of people. Also, some other projects decided to still support both (lego for instance).
Ideally, a log message could inform the user the API Key is deprecated in favor of PAT.

[RealOrangeOne]

I've pushed some tweaks. The API isn't perfect, as we still prompt for both values even though only one is used - perhaps there should be 2 constructors instead?

[nlewo]

@RealOrangeOne I think we should keep a single constructor since we are supposed to remove the API_KEY method at some point: i don't think we should avoir introducing a new constructor to remove it in the future.

we still prompt for both values even though only one is used

Sorry, i don't understand what do you mean by "prompting both values". Is it the king library which prompt for both?

From my point of view, it would be good enough to fail in the constructor when both the API key and the PAT are empty strings (and displaying a help message suggesting to use a PAT).

[RealOrangeOne]

we still prompt for both values

My point here was mostly that the constructor takes 2 values, even though we only use one. I like the idea of showing an error when both are provided - I think that covers the usability issues nicely.

[RealOrangeOne]

@nlewo over to you!

[nlewo]

FYI, i renamed Token to PersonalAccessToken