Skip to content
This repository was archived by the owner on Dec 11, 2024. It is now read-only.

(MINOR) Rework c2patool parameters #53

Merged
merged 11 commits into from
Aug 18, 2022
Merged

Conversation

gpeacock
Copy link
Contributor

@gpeacock gpeacock commented Aug 18, 2022

Changes in this pull request

Viewing files works the same, Manifest creation parameters are different.
source file must always be passed
manifest config must be passed with the new -m/manifest option
won't overwrite files without the new -f/force flag
Code restructure
-v -s combo no longer deletes the sidecar

Checklist

  • This PR represents a single feature, fix, or change.
  • All applicable changes have been documented.
  • Any TO DO items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.

mauricefisher64 and others added 7 commits August 12, 2022 10:21
…patool into external_manifest

* 'external_manifest' of https://github.com/contentauth/c2patool:
  update c2pa version, clippy fixes

# Conflicts:
#	Cargo.toml
source file must always be passed
manifest config is always an option
won't overwrite files without -force flag
Code restructure
@scouten-adobe scouten-adobe changed the title Parameter rework (MINOR) Rework c2patool parameters Aug 18, 2022
@scouten-adobe scouten-adobe requested a review from crandmck August 18, 2022 15:29
@scouten-adobe
Copy link
Collaborator

Asking for review from @crandmck given substantial documentation changes.

Add reminder about version changes being generated automatically on publish.
Copy link
Collaborator

@scouten-adobe scouten-adobe left a comment

Choose a reason for hiding this comment

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

I backed out the manual version upgrade and retitled the PR so that it will generate a minor version change when we next publish. Other than that, 👍.

Copy link
Collaborator

@crandmck crandmck left a comment

Choose a reason for hiding this comment

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

One small suggestion, otherwise LGTM.

Copy link
Contributor

@mauricefisher64 mauricefisher64 left a comment

Choose a reason for hiding this comment

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

👍

@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2022

Codecov Report

Merging #53 (17e2481) into main (314b4bf) will decrease coverage by 2.39%.
The diff coverage is 65.83%.

@@            Coverage Diff             @@
##             main      #53      +/-   ##
==========================================
- Coverage   75.00%   72.60%   -2.40%     
==========================================
  Files           3        3              
  Lines         216      230      +14     
==========================================
+ Hits          162      167       +5     
- Misses         54       63       +9     
Impacted Files Coverage Δ
src/main.rs 60.00% <52.87%> (-10.18%) ⬇️
src/manifest_config.rs 80.28% <80.28%> (ø)
src/signer.rs 93.18% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@gpeacock gpeacock merged commit cede246 into main Aug 18, 2022
@gpeacock gpeacock deleted the gpeacock/parameter_change branch August 18, 2022 20:05
@crandmck crandmck mentioned this pull request Aug 18, 2022
3 tasks
scouten-adobe pushed a commit to contentauth/c2pa-rs that referenced this pull request Dec 10, 2024
* Change in command line parameters for adding manifests, please review README
* Support for external/remote manifests
* update c2pa version
* Updated Readme with new options
* Change in command line parameters for adding manifests, please review README
** source file must now always be passed on command line
** manifest config now requires a -m --manifest option
** won't overwrite files without -force flag
Code restructure
Co-authored-by: Maurice Fisher <mfisher@adobe.com>
Co-authored-by: Eric Scouten <scouten@adobe.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.

5 participants