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

AzureDevOpsDsc: Increase Azure DevOps API timeout and other minor fixes (fixes #5, #10, #11, #12 and #25) #28

Conversation

SphenicPaul
Copy link
Contributor

@SphenicPaul SphenicPaul commented Jan 28, 2021

Pull Request (PR) description

Includes a number of minor fixes to close some small/minor issues:

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry to the change log under the Unreleased section of the
    file CHANGELOG.md. Entry should say what was changed and how that
    affects users (if applicable), and reference the issue being resolved
    (if applicable).
  • Resource documentation updated in the resource's README.md.
  • Resource parameter descriptions updated in schema.mof.
  • Comment-based help updated, including parameter descriptions.
  • Localization strings updated.
  • Examples updated.
  • Unit tests updated. See DSC Community Testing Guidelines.
  • Integration tests updated (where possible). See DSC Community Testing Guidelines.
  • Code changes adheres to DSC Community Style Guidelines.

This change is Reviewable

@SphenicPaul SphenicPaul changed the title AzureDevOpsDsc: Increase Azure DevOps API timeout and other minor fixes (fixes #5, #10, '11 and #25) AzureDevOpsDsc: Increase Azure DevOps API timeout and other minor fixes (fixes #5, #10, #11, #12 and #25) Jan 28, 2021
@SphenicPaul SphenicPaul marked this pull request as ready for review January 28, 2021 19:20
@SphenicPaul
Copy link
Contributor Author

@johlju - Shout if you've any questions with this. Nothing too major within these changes.

The integration tests ran OK locally against my Azure DevOps instance so I'd expect them to run OK when merged. Additionally, the API timeout is now 5 minutes so that should resolve the intermittent, integration failures.

I might look at the SQL Server (2019) and Azure DevOps Server (2020) installs (on build server as part of integration tests) for the next PR.

Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

Reviewed 12 of 12 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @SphenicPaul)

a discussion (no related file):
Now that we are fixing enums, can we remove these?

# This enum is re-defined here so it is recognised by 'Import-DscResource' (which won't pre/post-parse the 'using' statements)
#enum Ensure
#{
# Present
# Absent
#}


Copy link
Contributor Author

@SphenicPaul SphenicPaul left a comment

Choose a reason for hiding this comment

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

Reviewable status: 12 of 16 files reviewed, 1 unresolved discussion (waiting on @johlju)

a discussion (no related file):

Previously, johlju (Johan Ljunggren) wrote…

Now that we are fixing enums, can we remove these?

# This enum is re-defined here so it is recognised by 'Import-DscResource' (which won't pre/post-parse the 'using' statements)
#enum Ensure
#{
# Present
# Absent
#}

Done.

I've added some comments/properties on the three, base classes also.


Copy link
Contributor Author

@SphenicPaul SphenicPaul left a comment

Choose a reason for hiding this comment

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

Reviewable status: 12 of 16 files reviewed, 1 unresolved discussion (waiting on @johlju)

a discussion (no related file):

Previously, SphenicPaul (Paul J. Wilcox) wrote…

Done.

I've added some comments/properties on the three, base classes also.

...and also added comments/headers for the AzDevOpsProjectclass/resource.


Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @SphenicPaul)

@johlju johlju merged commit f1d8097 into dsccommunity:main Jan 29, 2021
@SphenicPaul SphenicPaul deleted the pjw-issue25-fixFailingIntegrationTestTimeoutsPlusOthers branch January 29, 2021 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment