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

return concrete types! #34810

Closed
wants to merge 1 commit into from
Closed

return concrete types! #34810

wants to merge 1 commit into from

Conversation

led0nk
Copy link
Contributor

@led0nk led0nk commented Aug 22, 2024

concrete/iface

Description:
As requested this PR removes the unnecessary use of

func (s *azureScraper) getArmClient() (armClient, error) 
func (s *azureScraper) getMetricsDefinitionsClient() (metricsDefinitionsClientInterface, error)
func (s *azureScraper) GetMetricsValuesClient() (metricsValuesClient, error)

Those methods returned interfaces which later got implemented by fields of the *azureScraper.
Now we directly assign them with their concrete types, which were handled through their interfaces before. The implementation of errorhandling was already done earlier.

Link to tracking Issue:

Testing:
Testing was done through the given scraper_test.go.

Documentation:

Copy link
Contributor

github-actions bot commented Sep 6, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

concrete/iface

interface fmt
@github-actions github-actions bot removed the Stale label Sep 7, 2024
@led0nk led0nk marked this pull request as ready for review September 7, 2024 14:02
@led0nk led0nk requested a review from a team September 7, 2024 14:02
@led0nk
Copy link
Contributor Author

led0nk commented Sep 7, 2024

image

do we solve this in this PR or do we have to raise another one?

@nslaughter
Copy link
Contributor

nslaughter commented Sep 20, 2024

do we solve this in this PR or do we have to raise another one?

IMO this is a separate issue / PR.

Otherwise, this PR looks great and cleans the code.

Copy link
Contributor

@nslaughter nslaughter left a comment

Choose a reason for hiding this comment

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

I have reviewed the deletions and approve providing for checks passing. This PR deletes unnecessary code and replaces usage with essential code.

Filling out the PR comment may be important for getting approvers' review.

@led0nk
Copy link
Contributor Author

led0nk commented Sep 20, 2024

Filling out the PR comment may be important for getting approvers' review.

I will take care of this, thanks

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 11, 2024
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Oct 25, 2024
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.

[receiver/azuremonitor] Return concrete types
3 participants