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.

Sign up for GitHub

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

Jump to bottom

Feat #226: Implement Confirm-Commit and Confirm-Push Flags for Improved User Experience #227

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

Feat #226: Implement Confirm-Commit and Confirm-Push Flags for Improved User Experience #227

wants to merge 8 commits into from

Conversation

malpou
Copy link
Contributor

@malpou malpou commented Jul 20, 2023 •
edited

In this pull request, we have introduced new functionality to streamline the user's interaction with the CLI tool. This enhancement focuses on two main aspects: committing and pushing actions, which can now be confirmed upfront with the use of flags.

Changes:

  • Update in cli.ts: Introduced confirmCommitFlag and confirmPushFlag as command-line flags. These flags provide a means for users to confirm commit and push actions at the start of the process. As a result, they will no longer need to confirm these actions manually each time they occur, greatly improving the user experience.
  • Enhancement in commit.ts: Added support for the newly introduced confirmCommitFlag and confirmPushFlag. If the flags are set to true, the commit and push actions proceed automatically without requiring user confirmation. If the flags are false or not provided, the user is prompted to confirm these actions. This modification ensures seamless integration of the new flags with existing functionalities.

We believe that this update will significantly improve the experience of our users by making the commit and push processes more streamlined and user-friendly.

Issue #226

@malpou
fix(cli.ts): add flags for confirming commit and push actions to impr…
72cdda2 
…ove user experience

feat(commit.ts): add support for confirm-commit and confirm-push flags to allow users to skip confirmation prompts
@malpou malpou mentioned this pull request Jul 20, 2023
Open
@ionutz89
Copy link

Any update about this feature?

@malpou
Copy link
Contributor Author

malpou commented Jul 27, 2023

@ionutz89 Still waiting for someone to take a look at my PR

@dev-mtjoy
Copy link

+1

dev-mtjoy
dev-mtjoy reviewed Jul 27, 2023
View reviewed changes
Copy link

@dev-mtjoy dev-mtjoy 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 to me!

@dev-mtjoy
Copy link

@di-sukharev ?

sebastienfi
sebastienfi reviewed Aug 14, 2023
View reviewed changes
Copy link
Contributor

@sebastienfi sebastienfi left a comment

Choose a reason for hiding this comment

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

LGTM

@di-sukharev
Copy link
Owner

will get to it soon <3

@di-sukharev
Copy link
Owner

@malpou what is the example to use it, is it like oco --confirm-commit?

@charanjit-singh
Copy link

Any update? Need this badly

@malpou
Copy link
Contributor Author

malpou commented Aug 29, 2023

@malpou what is the example to use it, is it like oco --confirm-commit?

Yes, that's the though it was mentioned in an issue #226

@di-sukharev
Copy link
Owner

@malpou i dont think that so long message is really an optimal ux, i would go with a shorter flag like oco -f (make sure -f is not in git itself, i dont remember)

Christophe-Chantraine-djm-digital
Christophe-Chantraine-djm-digital reviewed Sep 4, 2023
View reviewed changes
Copy link

Choose a reason for hiding this comment

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

That sounds good for me

@Christophe-Chantraine-djm-digital
Copy link

@malpou i dont think that so long message is really an optimal ux, i would go with a shorter flag like oco -f (make sure -f is not in git itself, i dont remember)

Agree with that, but what do you mean by "not in git itself" ? Are you talking about this command, for example:
git push -f ? If yes, this param exists on the git push.

@di-sukharev
Copy link
Owner

@Christophe-Chantraine-djm-digital, yep i meant what you described, but i think if we do oco --f it would still be first processed by opencommit and we can manage the expected behaviour.

So, fixing the conflict, making the flags shorter (--f is good), then building npm run build and testing locally npm start --f should be all. I will merge when you are sure all is tested <3

@malpou
Merge commit 'ca049e4b5d5eb16bb2ee67cfd5c1f8e9ebdacd0c' into add-conf…
8978a4a 
…irm-commit-and-push-args
@malpou
🔧 chore(cli.ts): refactor flags in cli command to improve readability…
e4e19dc 
… and maintainability

🔧 chore(cli.ts): refactor commit function call to use the new flag names for better clarity
@malpou
🔧 chore(cli.ts): remove unnecessary default values for cc and cp flag…
2c13738 
…s in the CLI configuration

🔧 chore(cli.ts): remove console.log statement used for debugging
@malpou
🔧 chore(cli.ts): remove unnecessary console.log statement
e64b788 
🔧 chore(cli.ts): simplify flags definition by removing explicit type declarations
@malpou
Copy link
Contributor Author

malpou commented Sep 6, 2023 •
edited

The flags are suitable and tested now! @di-sukharev

Notes:

  • When testing you need to use npm start -- --cm --cp else params are not parsed to our CLI tool.
  • Flags in cleye can't be shorter than 2 chars.

Flags:

  • --cm : Confirm commit message
  • --cp: Confirm push

Readme updated aswell

@malpou
🔧 chore(README.md): add documentation for optional flags in oco command
6034b37 
🔧 chore(cli.ts): rename cc flag to cm flag for better semantics
🔧 chore(commit.ts): rename confirmCommitFlag to confirmCommitMessageFlag for better semantics
@malpou
📝 docs(README.md): update flag descriptions for oco command
f7ffdf0 
- Update description for `--cm` flag to clarify that it skips the confirmation step of the generated commit message
- Update description for `--cp` flag to clarify that it confirms the push to remote after committing, including choosing upstream/origin in case of a forked repo
@malpou
Copy link
Contributor Author

malpou commented Sep 12, 2023

@di-sukharev
Any blockers on this?

@github-actions
Copy link

Stale pull request message

@github-actions github-actions bot closed this Oct 30, 2023
@di-sukharev di-sukharev reopened this Feb 28, 2024
@di-sukharev
Copy link
Owner

di-sukharev commented Feb 28, 2024 •
edited

@malpou im so sorry this got closed, do you want to merge it? if so — please solve the conflict and i will get to it asap

@di-sukharev
Copy link
Owner

@malpou i've solved the readme conflict, could you check im not missing anything please and lets merge it!

@dangnhdev
Copy link

@malpou please check it so we can merge it quickly 🚀

@di-sukharev
Copy link
Owner

@malpou please solve the conflicts, i will merge, test and ship in the next version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebastienfi sebastienfi sebastienfi left review comments

@Christophe-Chantraine-djm-digital Christophe-Chantraine-djm-digital Christophe-Chantraine-djm-digital left review comments

@dev-mtjoy dev-mtjoy dev-mtjoy left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@malpou @ionutz89 @dev-mtjoy @di-sukharev @charanjit-singh @Christophe-Chantraine-djm-digital @dangnhdev @sebastienfi