feat: add config package & optimize dir package #90
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JeyJeyGao
force-pushed
the
feature/directory_structure
branch
3 times, most recently
from
c1110c4
to
921fa2a
Compare
JeyJeyGao
added a commit
to JeyJeyGao/notation-go
that referenced
this pull request
Jul 22, 2022
config package migrate config package from notation to notation-go split out the signingkeys information from config.json to be a signingkeys.json file migrate config/util.go from notation to notation-go add reusable Load() & Save() function to manage different configuration file dir package optimize dir/path.go API make the file/directory name as a constent in dir/path.go Signed-off-by: Junjie Gao <43160897+JeyJeyGao@users.noreply.github.com>
JeyJeyGao
force-pushed
the
feature/directory_structure
branch
from
921fa2a
to
7cfcf89
Compare
binbin-li
reviewed
Jul 22, 2022
|
typo in pr description: |
JeyJeyGao
force-pushed
the
feature/directory_structure
branch
2 times, most recently
from
ca98605
to
21f63ea
Compare
JeyJeyGao
force-pushed
the
feature/directory_structure
branch
7 times, most recently
from
eb92e12
to
48fd391
Compare
|
LGTM |
binbin-li
reviewed
Jul 27, 2022
|
lgtm |
JeyJeyGao
force-pushed
the
feature/directory_structure
branch
2 times, most recently
from
cfedfc5
to
822a12b
Compare
config package migrate config package from notation to notation-go split out the signingkeys information from config.json to be a signingkeys.json file migrate config/util.go from notation to notation-go add reusable Load() & Save() function to manage different configuration file dir package optimize dir/path.go API make the file/directory name as a constent in dir/path.go Signed-off-by: Junjie Gao <43160897+JeyJeyGao@users.noreply.github.com>
JeyJeyGao
force-pushed
the
feature/directory_structure
branch
from
822a12b
to
5d99707
Compare
Codecov Report
@@ Coverage Diff @@
## main #90 +/- ##
==========================================
+ Coverage 66.21% 68.10% +1.89%
==========================================
Files 32 35 +3
Lines 1835 1919 +84
==========================================
+ Hits 1215 1307 +92
+ Misses 501 490 -11
- Partials 119 122 +3
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
JeyJeyGao
force-pushed
the
feature/directory_structure
branch
2 times, most recently
from
ebe0adf
to
d8e4a67
Compare
shizhMSFT
reviewed
Jul 29, 2022
JeyJeyGao
force-pushed
the
feature/directory_structure
branch
from
d8e4a67
to
61104be
Compare
Signed-off-by: Junjie Gao <43160897+JeyJeyGao@users.noreply.github.com>
JeyJeyGao
force-pushed
the
feature/directory_structure
branch
2 times, most recently
from
c6389c1
to
730bd31
Compare
binbin-li
reviewed
Jul 29, 2022
JeyJeyGao
force-pushed
the
feature/directory_structure
branch
2 times, most recently
from
0e805cb
to
9711344
Compare
|
lgtm |
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
JeyJeyGao
force-pushed
the
feature/directory_structure
branch
from
9711344
to
08d6a49
Compare
binbin-li
approved these changes
Aug 2, 2022
rgnote
reviewed
Aug 2, 2022
| "github.com/opencontainers/go-digest" | ||
| ) | ||
|
|
||
| const ( | ||
| // CertificateExtension defines the extension of the certificate files | ||
| CertificateExtension = ".crt" |
There was a problem hiding this comment.
certificates can be in .pem, .cer, .der as well. If this is meant to be used for local certificates which are created using Notation, then we should rename this to LocalCertificateExtension
There was a problem hiding this comment.
LocalCertificateExtension is better. I will rename it.
| ConfigFile = "config.json" | ||
|
|
||
| // KeyExtension defines the extension of the key files | ||
| KeyExtension = ".key" |
| func checkError(err error) { | ||
| // if path does not exist, the path can be used to create file | ||
| if err != nil && !errors.Is(err, fs.ErrNotExist) { | ||
| panic(err) |
There was a problem hiding this comment.
Do we really need to panic here? Returning errors seems to be a better option for Go libraries per https://stackoverflow.com/a/35413011
priteshbandi
pushed a commit
to priteshbandi/notation-go
that referenced
this pull request
Aug 9, 2022
Signed-off-by: Junjie Gao <junjiegao@microsoft.com> Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
config package
Config package includes the necessary configuration information for notation-go library. Previously, it is in notation project. It is time to migrate the config package to notation-go.
dir package