Made new-user commit its changes #14
Merged
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
| // Commit changes | ||
| let signature = Signature::now("Proton Lights", "[email protected]").unwrap(); | ||
| let msg = format!("Adding {} as new user", name); | ||
| let pf_path = Path::new("Protonfile.json").as_ref(); |
There was a problem hiding this comment.
Is there a reason you do an as_ref() here instead of just doing a & reference on line 31? Or is that not the same?
There was a problem hiding this comment.
No reason. It works the same. Good catch
|
It looks good aside from the things I picked out. |
| @@ -18,27 +18,27 @@ pub fn commit_file<P: AsRef<Path>>( | |||
| ) -> Result<(), Error> { | |||
|
|
|||
| let repo = try!(get_repo_from_path(repo_path)); | |||
|
|
|||
| let _ = repo.index() | |||
| let tree_oid = try!(repo.index() | |||
There was a problem hiding this comment.
Okay, so I don't really like these changes. It seems like it got more complex. I think how you had it before was better. The only change I was looking for was that line 22 changed from a discarding assignment to a return.
I really dislike the code pattern where we have a bunch of stuff tryed! and then ended with an Ok(()). I think you should just get rid of the last try! and change it to a return with a .map(|_| OK(())). What do you think?
There was a problem hiding this comment.
Agreed, it's ugly. Didn't know how to make the map work before, but figured it out. Lmk if still could be improved. Some/all of the try!() statements have to stay, in order to keep repo in scope.
|
Ok, lets go with this. I get this is an older commit, but it touches less things, so rebase this on top of iss8, since that's already gotten an approval. |
bookdude13
added a commit
that referenced
this pull request
May 20, 2017
* Added name to authors (#4) * Name (#7) * Added Vlad to authors list (#10) * Overhaul of structure to use DAOs and database for backend storage (#11) * Initial database implementations (#12) * Updated README (#14) * Working code for 2016 show (#19) * Created initial 'it works' test * Restructured into separate tests. 'is-working' tests complete? * Restructured 'all working' test * Added more tests. Reorganized again * One more test * Added test for seq sec editor added on perm change. Implemented; passing * All tests implemented except working and 0 sections * All tests passing * Restructured so sequence not returned on resection * Converted main code to new permissions api * Tests using new api. Removed check for sequence section editor. Some tests failing * All tests passing * Removed editor from sequence_section type * Tests now want seq sec in range [0, numSections) * SeqSec now 0-indexed. Removed last traces of editor field * Updated README and fixed error in main * Added checks * Moving towards Fixtures instead of 2d blocks of sequence data * Restructured to use DAOs and database setup instead of storing in flat files * Added initial database backup * Postgres connection working in channel dao * Generalized common functionality * Updated names, public instead of private key, uid for list-permissions * Init project returns generated public key * Updated database definition * Looser Cargo package versions for upgrading/good documentation * Adding initial user mostly implemented. Util function to gen pub/priv keys * Store both pub/priv key. Trim before compare. Get uid from key started * Permission handling stuff. PermissionEnum is string now for simplicity * new-user fully implemented * A little cleanup of tests; moving towards using again * Removed unused imports. New return type for get-uid * PermissionEnum updated. Preliminary work on add_sequence done * new_sequence working? * get_sequence command added to main * new-layout function set up * Reading layout from file. Locations and rotations checked * Adding layout implemented. Transactions/removing not implemented * Layout as of 11/21/2016 added * Project in db behind DAO now as well. General cleanup * set-sequence-layout working * Existential dao functions * Channel data now in separate table * Moved db backups to separate folder * get-layout-id command added, not implemented * Finished implementing add-sequence * get-playlist-data implemented * new-vixen-sequence working. Nullable values changed to options. * Updated Cargo.toml * Added seqid to playlist output * Changed the format of playlist data * Added music file to playlist outputted data * Made functional again after rebase * Added sequence duration to Sequence object for accurate num_frames calc * Added internal channel to Channel type and layout reading * Patching stuff * Added current db backup with internal channel * Patching working now; hackish fix * Added patch file * Updated layout and patch * Working show backed up * Stopped copying music over * Using postgres FUNCTION for patching * Added DB with FUNCTION in last commit added * New patch * Added newer patches * main.rs alphabetized. Added get-project command * Cleaned up warnings * More verbose patching * Added SQL to reset database to clean state * Fixed error with reading file as string * WORKINGgit st! * Working show backup * Adding admin key for 1202 backup * Actual working show backup (whoops) * add-sequence turned into insert-sequence * Udated patch * Print statemets for verbosity * 12_03 patch * Different format for outputting playlist data * Removed public keys from repo * New sequences don't copy their music files to a Music/ directory * Version now 0.19.1 as per SemVer (#33) * Adding beginning of testing framework post-overhaul (#32) * Documentation (#28) * Removed rebase artifacts. Updated version in Cargo.lock (#34) * Permissions checks moved from the second-tier functions to main.rs (#35) * Permissions checks moved from the second-teir functions to main.rs * Fixes issues to moving permissions check * Updated version numbers for dependencies (removing wildcards) (#36)
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.
Addresses #12.