Skip to content
This repository has been archived by the owner on Apr 7, 2024. It is now read-only.

feat: Add 'MemoryStore' #92

Merged
merged 6 commits into from
Sep 11, 2023
Merged

feat: Add 'MemoryStore' #92

merged 6 commits into from
Sep 11, 2023

Conversation

uanid
Copy link
Contributor

@uanid uanid commented Sep 2, 2023

Features:

  1. Add MemoryStore

Reference Issue: #89

@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2023

Codecov Report

Merging #92 (3984aa5) into main (f971976) will increase coverage by 0.78%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main      #92      +/-   ##
==========================================
+ Coverage   76.08%   76.86%   +0.78%     
==========================================
  Files           7        8       +1     
  Lines         414      428      +14     
==========================================
+ Hits          315      329      +14     
  Misses         77       77              
  Partials       22       22              
Files Changed Coverage Δ
memory_store.go 100.00% <100.00%> (ø)

inmemory_store.go Outdated Show resolved Hide resolved
inmemory_store.go Outdated Show resolved Hide resolved
Signed-off-by: Uanid <uanid@outlook.com>
Signed-off-by: Uanid <uanid@outlook.com>
Signed-off-by: Uanid <uanid@outlook.com>
Copy link
Member

@Wwwsylvia Wwwsylvia left a comment

Choose a reason for hiding this comment

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

LGTM with nit

inmemory_store.go Outdated Show resolved Hide resolved
@Wwwsylvia
Copy link
Member

Wwwsylvia commented Sep 8, 2023

@shizhMSFT Could you take a look as well?

Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM with suggestions

)

// InMemoryStore is a store that keeps credentials in memory.
type InMemoryStore struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will it be better to name it MemoryStore instead of InMemoryStore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That might be a good name. Changed it to MemoryStore.

Co-authored-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Minuk Song <uanid@outlook.com>
@uanid uanid changed the title feat: Add 'InMemoryStore' feat: Add 'MemoryStore' Sep 8, 2023
Signed-off-by: Uanid <uanid@outlook.com>
memory_store.go Outdated Show resolved Hide resolved
Signed-off-by: Uanid <uanid@outlook.com>
Copy link
Member

@Wwwsylvia Wwwsylvia left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@Wwwsylvia Wwwsylvia merged commit 8b76a43 into oras-project:main Sep 11, 2023
7 checks passed
@uanid uanid deleted the feature/inmemory-store branch September 11, 2023 07:55
@Wwwsylvia
Copy link
Member

Merged. Thank you @uanid again for contributing this!

Wwwsylvia pushed a commit to oras-project/oras-go that referenced this pull request Sep 20, 2023
Features:
-  Add `MemoryStore`

I submitted a pull request for MemoryStore to `oras-credentials-go` a
week ago. However, the repository has since been deprecated. I've
created an identical PR to [this
one](oras-project/oras-credentials-go#92).

Reference Issue:
[Here](oras-project/oras-credentials-go#89)

Signed-off-by: Uanid <uanid@outlook.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants