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

docs: update readme to include CLI #32

Merged
merged 2 commits into from
Apr 7, 2024
Merged

docs: update readme to include CLI #32

merged 2 commits into from
Apr 7, 2024

Conversation

martincarapia
Copy link
Contributor

@martincarapia martincarapia commented Apr 4, 2024

Description

Update README.md to include CLI.

Related Issue

Fixes #29

Type of Change

Please mark the appropriate option below to describe the type of change your pull request introduces:

  • Documentation update

Checklist

  • My pull request has a clear title and description.
  • I have used semantic commit messages.
    Examples: "fix: Fixed foobar bug", "feat(accounts): Added foobar feature".
  • I have added/updated the necessary documentation on README.md.
  • I have added appropriate test cases (if applicable) to ensure the changes are functioning correctly.

By submitting this pull request, I confirm that I have read and complied with the contribution guidelines of this project.

@aj3sh aj3sh requested review from aj3sh and subashcs and removed request for aj3sh April 4, 2024 16:38
Copy link
Member

@aj3sh aj3sh left a comment

Choose a reason for hiding this comment

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

@martincarapia, I have added a few suggestions. Also, please include the text Closes issue #29 in the commit body instead of the commit header.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@martincarapia martincarapia changed the title docs: update readme to include CLI. Closes issue #29 docs: update readme to include CLI Apr 4, 2024
@aj3sh aj3sh requested a review from sugat009 April 4, 2024 17:04
@aj3sh
Copy link
Member

aj3sh commented Apr 4, 2024

Sorry, I forgot to mention that we need to install the commitlint before using it.

pip install commitlint

@aj3sh
Copy link
Member

aj3sh commented Apr 5, 2024

Your commit still contains the Closes issue #29 text in the header. Please edit your commit message.

@martincarapia
Copy link
Contributor Author

Alright fixed. Everything should be good.

@aj3sh
Copy link
Member

aj3sh commented Apr 5, 2024

Alright fixed. Everything should be good.

Now your PR has 7 commits. This can be fixed if you rebase your branch from the main branch.

@martincarapia
Copy link
Contributor Author

Rebased.

@martincarapia martincarapia requested a review from aj3sh April 5, 2024 23:52
@aj3sh
Copy link
Member

aj3sh commented Apr 6, 2024

@martincarapia, You somehow accidentally added some unnecessary commits to your branch. Consequently, your PR has four commits and two of them already exist in the main branch. I have added an image for the clarification.

However, this can be fixed using an interactive rebase. Make sure your branch updates with the main branch.

git fetch origin main
git rebase origin/main

Once the rebase is complete, you can use the interactive rebase.

git rebase -i 8761e42

where 8761e42 is the last commit of the main branch.

Using interactive rebase, you can delete unnecessary commits, squash two commits, reword the commits, and more.

Moreover, your commit header cannot end with period (.), and the best practice is to keep one commit for one PR.

Screenshot 2024-04-06 at 9 56 20 AM

Reference:
https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History
https://medium.com/swlh/tutorial-on-interactive-rebasing-e3f49505b2ce

@martincarapia
Copy link
Contributor Author

Thank you for your clarification, and thank you for your patience. I'm new to all of this.

Copy link
Member

@aj3sh aj3sh left a comment

Choose a reason for hiding this comment

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

Looks good.

@sugat009 sugat009 merged commit 1e6587d into opensource-nepal:main Apr 7, 2024
3 checks passed
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.

Doc for CLI usage
3 participants