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

Refactoring pkg Archivista API and include new http client for Archivista #424

Merged
merged 3 commits into from
Jan 24, 2025

Conversation

kairoaraujo
Copy link
Collaborator

What this PR does / why we need it

  • Refactor package Archivista API
  • Add package Archivista HTTP Client

Acceptance Criteria Met

  • Docs changes if needed
  • Testing changes if needed
  • All workflow checks passing (automatically enforced)
  • All review conversations resolved (automatically enforced)
  • DCO Sign-off

Special notes for your reviewer:

Copy link

codecov bot commented Nov 20, 2024

Codecov Report

Attention: Patch coverage is 37.58389% with 93 lines in your changes missing coverage. Please review.

Project coverage is 1.82%. Comparing base (a035c62) to head (75a5462).
Report is 251 commits behind head on main.

Files with missing lines Patch % Lines
pkg/http-client/client.go 37.75% 58 Missing and 3 partials ⚠️
pkg/api/download.go 18.18% 26 Missing and 1 partial ⚠️
pkg/api/graphql.go 66.66% 2 Missing and 1 partial ⚠️
pkg/api/upload.go 75.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #424       +/-   ##
==========================================
- Coverage   82.40%   1.82%   -80.58%     
==========================================
  Files          10     124      +114     
  Lines         358   33571    +33213     
==========================================
+ Hits          295     612      +317     
- Misses         43   32897    +32854     
- Partials       20      62       +42     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kairoaraujo kairoaraujo force-pushed the refactor_api_http_client branch 2 times, most recently from b9f3d19 to e41b846 Compare November 20, 2024 09:55
@kairoaraujo kairoaraujo force-pushed the refactor_api_http_client branch from e41b846 to b8e39fd Compare December 2, 2024 11:31
@kairoaraujo kairoaraujo requested a review from jkjell December 2, 2024 11:39
@kairoaraujo kairoaraujo force-pushed the refactor_api_http_client branch from b8e39fd to 59caa5a Compare December 3, 2024 14:59
Copy link
Member

@jkjell jkjell left a comment

Choose a reason for hiding this comment

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

Looks good so far! I know there's still some outstanding discussion on naming but, after that, I'll give a closer look to the content of the testing.

pkg/api/structs.go Show resolved Hide resolved
pkg/http-client/client.go Show resolved Hide resolved
)
}

func (ac *ArchivistaClient) GraphQLQueryIface(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this function intended to be used by a user? It would probably be helpful to have some descriptions above the functions, that way they will show up in https://pkg.go.dev/

Copy link
Collaborator

Choose a reason for hiding this comment

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

these look great!

kairoaraujo and others added 3 commits January 23, 2025 11:17
add a pkg with a HTTP client for Archivista using the pkg API

Signed-off-by: Kairo Araujo <[email protected]>
@kairoaraujo kairoaraujo force-pushed the refactor_api_http_client branch from 59caa5a to 75a5462 Compare January 23, 2025 10:26
Copy link
Member

@jkjell jkjell left a comment

Choose a reason for hiding this comment

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

Look great! Thanks Kairo. 🙌

@kairoaraujo kairoaraujo merged commit 51bb817 into in-toto:main Jan 24, 2025
11 of 13 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