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

AppVeyor: Handle v2 tokens #8830

Merged
1 commit merged into from Feb 11, 2021

Conversation

gerhardol
Copy link
Member

Fixes #8828

Proposed changes

v2 tokens require that the account is prepended to the request..
This apparently is only required for quering repos for an account,
not normal info.

  • Only query for project names for a configured account if no
    account/repo at all is specified
    Illegal repos will not report any builds anyway.
    This allows a AppVeyor username/token at the same time as multiple
    repos are used. For instance can gitextensions/gitextensions builds
    be seen at the same time as user private builds

  • Allow configuring of account/repo also if accountName is set

  • Less aggressive requery of running AppVeyor jobs

  • Refactor

    • remove static variables/functions
    • Handle projectId (and name) as account/repo consistently
    • Use local functions (and inline code)
    • Remove internal data class Project (the module global is no longer needed)

Test methodology

Tests updated
Manual


✒️ I contribute this code under The Developer Certificate of Origin.

@ghost ghost assigned gerhardol Feb 7, 2021
@codecov
Copy link

codecov bot commented Feb 7, 2021

Codecov Report

Merging #8830 (d55519f) into master (18616ed) will increase coverage by 0.05%.
The diff coverage is 33.84%.

@@            Coverage Diff             @@
##           master    #8830      +/-   ##
==========================================
+ Coverage   56.11%   56.16%   +0.05%     
==========================================
  Files         919      919              
  Lines       65524    65497      -27     
  Branches    11996    11992       -4     
==========================================
+ Hits        36766    36787      +21     
+ Misses      25756    25708      -48     
  Partials     3002     3002              
Flag Coverage Δ
production 43.33% <31.74%> (+0.07%) ⬆️
tests 94.44% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@RussKie
Copy link
Member

RussKie commented Feb 8, 2021

If it works - :shipit:

@gerhardol
Copy link
Member Author

If it works - :shipit:

I cannot test with v1 tokens, but v1 tokens have the same api so it should be the same.
It is of course possible that the cleanup logic breaks something, but I cannot think of any lost functionality (for accountName vs account/repo).
Some use cases are added though.

Plan to squash and initiate a merge tomorrow.
This should be considered for 3.5 too.

@gerhardol
Copy link
Member Author

will squash and rebase tonight

v2 tokens require that the account is prepended to the request..
This apparently is only required for quering repos for an account,
not normal info.

* Only query for project names for a configured account if no
account/repo at all is specified
Illegal repos will not report any builds anyway.
This allows a AppVeyor username/token at the same time as multiple
repos are used. For instance can gitextensions/gitextensions builds
be seen at the same time as user private builds

* Allow configuring of  account/repo also if accountName is set

* Less aggressive requery of running AppVeyor jobs

* Refactor
  * remove static variables/functions
  * Handle projectId/name as account/repo consistently
  * Use local functions (and inline code)
  * Remove internal data class Project (the module global is no longer needed)
@ghost
Copy link

ghost commented Feb 10, 2021

Hello @gerhardol!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Thu, 11 Feb 2021 20:36:01 GMT, which is in 1 day

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@gitextensions gitextensions deleted a comment Feb 10, 2021
@ghost ghost merged commit aabe00b into gitextensions:master Feb 11, 2021
@ghost ghost added this to the 3.6 milestone Feb 11, 2021
@gerhardol gerhardol deleted the feature/appveyor-v2-tokens branch February 11, 2021 20:44
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AppVeyor build integration does not work with v2 tokens
2 participants