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

Introduce run to execute user commands #179

Open
boonwj opened this issue Oct 24, 2019 · 11 comments
Open

Introduce run to execute user commands #179

boonwj opened this issue Oct 24, 2019 · 11 comments
Labels
documentation Create/Update documents good first issue Good for first time contributors help wanted Extra attention is needed new_feature New features
Milestone

Comments

@boonwj
Copy link
Contributor

boonwj commented Oct 24, 2019

Description

Users are currently able to configure command names that are used by 1build. In this situation, 1build's main action will take place instead of the user's command. An example of this is shown below.

[user@localhost 1build]$ 1build set init "echo hello"
[user@localhost 1build]$ 1build list
------------------------------------------------------------------------
project: gopi
before: export GO111MODULE=on
commands:
init | echo hello
------------------------------------------------------------------------
[user@localhost 1build]$ 1build init
'1build.yaml' configuration file already exists.

One way to deal with this will be for 1build to return an error when an invalid command name is found in the configuration. However, when additional commands are added by 1build in the future, it may conflict with a previously legal user configuration.

A specific 1build keyword such as run should be reserved to execute user-configured commands. This will allow future changes to 1build without affecting user configured commands.

Acceptance

  • 1build run [cmd1] should execute the user configured command
  • 1build run [cmd1] [cmd2] ... should execute the commands in sequence
  • README.md should reflect the change in usage
@gopinath-langote
Copy link
Owner

Hello @boonwj

Thank you for suggesting feature. This feature make sense as going further we introduce more and more 1build ccommands - this will segreagate user vs 1build commands.

Let's take this to next milestone.
Feel free to grab if you want to work on it.

@gopinath-langote gopinath-langote added new_feature New features documentation Create/Update documents good first issue Good for first time contributors help wanted Extra attention is needed labels Jan 29, 2020
@gopinath-langote gopinath-langote added this to To do in Feature Ideas / Bug tracker via automation Jan 29, 2020
@gopinath-langote gopinath-langote added this to the v1.5.0 milestone Jan 29, 2020
@gopinath-langote gopinath-langote pinned this issue Jan 29, 2020
@gopinath-langote
Copy link
Owner

@boonwj Did you get time to work on the issue?

@boonwj
Copy link
Contributor Author

boonwj commented Mar 31, 2020

Sorry, I forgot to follow up on this. I will work on it over the next few days.

@gopinath-langote gopinath-langote modified the milestones: v1.5.0, v1.6.0 Apr 1, 2020
@boonwj boonwj mentioned this issue Apr 5, 2020
2 tasks
@gopinath-langote
Copy link
Owner

@boonwj Hi,

I am assuming that, we are removing running user defined command by 1build xyz

  • the run command is optional and only useful when there is a conflict between 1build and user defined command.

The order of execution-

  1. 1build command
  2. user defined command
  3. if conflict - use 1build run xyz

Let me know if you need more clarification

@gopinath-langote
Copy link
Owner

The motivation behind this is to use as minimal command as possible.

And the idea behind this tool was to avoid long commands.

@boonwj
Copy link
Contributor Author

boonwj commented Apr 5, 2020

Oh I see what you mean. I misunderstood the intention of the feature.

In the above mentioned order of execution, the program's behavior may be inconsistent with what the user expects. For example, should 1build run be treated as an incomplete command and help message be displayed, or should the user-defined "run" be executed?

Would it perhaps be better for this functionality to be a flag --run instead of a keyword? Or perhaps, retain the existing logic, but just raise a warning when conflicting user defined commands are used?

wdyt?

@gopinath-langote
Copy link
Owner

Hi @boonwj ,

How about giving --run as flag when there is a conflict.

Order :

  1. Execute 1build command (delete,set, unset etc)
  2. If not 1build command - execute user-defined command from YAML file
  3. If we find the conflict – execute 1build command with warning message with help to run user-defined command is a priority by 1buildl --run command

Example yaml

project: Sample Web App
commands:
  - build: npm run build
  - unset: echo 'removing something'

Command: 1build unset build

– Here the output will be something like this:

Found the conflict of command name `unset` – Executing 1build command over project configured command. To execute project-specific command run `1build --run unset`

And then execute 1build command.

When passed 1build --run unset – run the command from the configuration.

WDYT?

@boonwj
Copy link
Contributor Author

boonwj commented Apr 7, 2020

I realised that this functionality in its current form would not be very useful.

Since any future conflicting command will still break a user's existing workflow (i.e. running 1build's command over user's defined). It is more likely that they change the conflicting keyword instead of adding a --run flag in their future runs.

Perhaps its better to raise a warning and continue executing user's commands first, or raise a conflicting keyword error and exit.

@gopinath-langote
Copy link
Owner

@boonwj I see. I got your point. Agree most likely user will change their command name instead of using flag.

Let's go ahead with a showing warning message and continue executing the user's command as of now.

With this we still have a chance to enhance this feature with --run flag in case users are more tends to do so.

WDYT?

@boonwj
Copy link
Contributor Author

boonwj commented Apr 9, 2020

Yeah lets go ahead with that. I will work on it over the next few days.

@gopinath-langote
Copy link
Owner

@boonwj thanks for signup. Looking forward to your PR.

@gopinath-langote gopinath-langote unpinned this issue May 9, 2020
@gopinath-langote gopinath-langote modified the milestones: v1.6.0, v1.7.0 Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Create/Update documents good first issue Good for first time contributors help wanted Extra attention is needed new_feature New features
Projects
Development

No branches or pull requests

2 participants