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

CLI Simplification #10

Closed
wants to merge 8 commits into from
Closed

CLI Simplification #10

wants to merge 8 commits into from

Conversation

scott113341
Copy link
Member

  • push-dir [remote] <dir>:<branch> [options...] syntax simplifying the CLI #7
  • Removed all flags except --preserve-local-temp-branch Lost changes when using --allow-unclean #9
  • Added --help, -h flag
  • Displays --help output when used incorrectly
  • Removed commit message formatting function
  • Added/removed tests
  • Moved tests into their own files to enable individual running

@rtsao
Copy link
Member

rtsao commented Jun 17, 2016

Force pushing workflow
I think force pushing to the remote branch should still be a supported option (perhaps a new meaning for --force).

Simple overrides
I guess --message was undocumented, but I think it's valid option, especially in the case of a force push workflow. Supporting something like --message="hello world %H" seems useful.

Local branch preservation
--preserve-local-temp-branch is kind of an oxymoron. Maybe it's best to treat the temp branch entirely as an implementation detail. Then if the user wants to create local branch, they can specify a --local-branch-name="cool-branch-%h" option, which creates a local branch with that name (and is preserved).

Dry runs
Git push supports --dry-run, which doesn't actually push anything. Maybe we can do something similar, which would be useful in combination with --local-branch-name for debugging purposes (mainly the pull + merge behavior when not force pushing)

var path = require('path');
var readme = path.join(__dirname, '..', 'README.md');
var readmeText = String(fs.readFileSync(readme));
var usage = /```usage([\s\S]*?)```/.exec(readmeText)[1].trim();
Copy link
Member

@rtsao rtsao Jun 17, 2016

Choose a reason for hiding this comment

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

😆 This is clever but is it worth it for DRYness? I'm not sure how often the usage docs will change in the future once we have API stability. Parsing the README at runtime seems kinda crazy.

Copy link
Member Author

Choose a reason for hiding this comment

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

push-dir|scott-dev⚡ ⇒ node benchmark.js
showUsageFromDisk x 53,288 ops/sec ±3.82% (79 runs sampled)
showUsageFromString x 76,362,229 ops/sec ±2.10% (76 runs sampled)
Fastest is showUsageFromString

Copy link
Member

Choose a reason for hiding this comment

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

fs.createReadStream('usage.txt').pipe(process.stdout)

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.

2 participants