Skip to content

Conversation

allank-fd
Copy link

@allank-fd allank-fd commented Apr 6, 2023

Fixes

Checklist

  • I have made a change to this repository, be it functionality, testing, documentation, spelling, or grammar.
  • I updated my branch with the master branch.
  • I have added the necessary testing to prove my fix is effective/my feature works (or I did not modify functionality).
  • I have added necessary documentation about the functionality in an appropriate .md file.
  • I have appropriately commented any code I have modified

Short description of what this PR does:

  • Added shorthand alias for cat c

@allank-fd allank-fd force-pushed the feature/add-short-hand-cat-alias branch from ae95040 to e11c7df Compare April 6, 2023 12:35
Copy link
Owner

@pimterry pimterry left a comment

Choose a reason for hiding this comment

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

It looks like you've reformatted and tweaked lots of unrelated code here, in both the tests and the core script. Can you remove all the changes that aren't required for this PR please, and squash the result back to a single commit? It's very useful to keep commits & PRs to a single set of changes.

I'm open to other changes too, but if you're doing that then it'd be helpful if you could open a separate PR for them, and provide some background on what you're trying to do there & why.

@allank-fd
Copy link
Author

@pimterry Thanks for the feedback, I've updated the PR to remove the formatting changes. Plus the small change to the Readme to keep it inline with the other command docs.

@allank-fd allank-fd requested a review from pimterry April 12, 2023 12:44
@pimterry pimterry merged commit ad1a146 into pimterry:master Apr 14, 2023
@pimterry
Copy link
Owner

Nice work, thanks! Merged 👍

pimterry added a commit that referenced this pull request Apr 14, 2023
pimterry added a commit that referenced this pull request Apr 14, 2023
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.

3 participants