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

Export Performance Improvement - Addresses #5615 #5629

Merged
merged 68 commits into from
Jan 17, 2025

Conversation

niwamo
Copy link
Contributor

@niwamo niwamo commented Jan 15, 2025

Pull Request (PR) description

Implements the changes described in #5615
I apologize in advance for the huge PR.
Tagging @FabienTschanz due to his expressed interest.

This Pull Request (PR) fixes the following issues

Implements the changes described in #5615

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 parameter descriptions added/updated in the schema.mof. NOT APPLICABLE
  • Resource documentation added/updated in README.md. NOT APPLICABLE
  • Resource settings.json file contains all required permissions. NOT APPLICABLE
  • Examples appropriately added/updated. NOT APPLICABLE
  • Unit tests added/updated.
  • New/changed code adheres to DSC Community Style Guidelines.

niwamo added 30 commits January 15, 2025 11:00
@FabienTschanz
Copy link
Contributor

I didn't check every and each resource, but from what I could tell, it all looks great. @niwamo Is there a specific reason that the Intune resources were not included? Just asking, I would take it up on myself to implement your changes in them as well. I know some pitfalls there from properties that would be missing (e.g. script content that is only available when fetching a single resource).

@niwamo
Copy link
Contributor Author

niwamo commented Jan 16, 2025

I didn't check every and each resource, but from what I could tell, it all looks great. @niwamo Is there a specific reason that the Intune resources were not included? Just asking, I would take it up on myself to implement your changes in them as well. I know some pitfalls there from properties that would be missing (e.g. script content that is only available when fetching a single resource).

No specific reason. For this PR, I focused on resources that I personally want to export, and I do not have a use case for the Intune modules.

@FabienTschanz
Copy link
Contributor

@niwamo Alright, I'll add the changes to the Intune resources.

Copy link
Member

@ykuijs ykuijs left a comment

Choose a reason for hiding this comment

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

LGTM

@ykuijs ykuijs merged commit 3dd01e3 into microsoft:Dev Jan 17, 2025
2 checks passed
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.

3 participants