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

Add CONTRIBUTING.md #246

Merged
merged 2 commits into from
Aug 14, 2019
Merged

Conversation

vitaliy-guliy
Copy link
Contributor

@vitaliy-guliy vitaliy-guliy commented Aug 8, 2019

What does this PR do?

Adds CONTRIBUTING.md file describing the development flow.
Updates devfile.yaml.
Adds formatter to package.json.

What issues does this PR fix or reference?

eclipse-che/che#14126

Still not working command running in container (need to found a solution):

  • package binaries
  • create a workspace

Before merging the link to chectl sources in the devfile will be changed to original git repo.

npm install -g pkg
workdir: /projects/chectl

- name: Package Binaries
Copy link
Collaborator

Choose a reason for hiding this comment

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

just to know, it's the deprecated binaries

it's oclif-dev pack now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I will try

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oclif-dev pack does not create executable file. It creates a package, which contains node with a set of javascript files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reworked

devfile.yaml Outdated
./run --help
workdir: /projects/chectl/bin

- name: Install Packager
Copy link
Collaborator

Choose a reason for hiding this comment

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

not required anymore (only for deprecated binaries)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed this command and updated a command to build packaged.
I will update it when it become clean how to use oclif-dev pack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

CONTRIBUTING.md Outdated
## Create workspace, clone sources

To create a workpace you can use [devfile](devfile.yaml).
For that you have to [install the latest version](https://www.eclipse.org/che/docs/che-7/che-quick-starts.html#installing-the-chectl-management-tool) of this chectl and create a workspace:
Copy link
Collaborator

Choose a reason for hiding this comment

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

it looks odd to install chectl when we clone chectl ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes and no. It's ok if we are explaining the user steps to create a workspace.
From other side it's extra information as user is already pretty experienced if he is going to contribute to chectl.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extra sentence has been removed.

CONTRIBUTING.md Outdated

#### 'dev' container

`chectl` is written on TypeScript. For that this container has preinstalled software to be able to build, test and launch `chectl`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
`chectl` is written on TypeScript. For that this container has preinstalled software to be able to build, test and launch `chectl`.
`chectl` is written in TypeScript. For that this container has preinstalled software to be able to build, test and launch `chectl`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks

CONTRIBUTING.md Outdated
- package binaries
- format sources

You can run commands in one of tree ways.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
You can run commands in one of tree ways.
You can run commands in one of three ways.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks

CONTRIBUTING.md Outdated

## Package binaries

### Install packager
Copy link
Collaborator

Choose a reason for hiding this comment

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

needs to be reworked. It's only using now oclif-dev pack

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reworked

@vitaliy-guliy vitaliy-guliy marked this pull request as ready for review August 13, 2019 07:38
@benoitf
Copy link
Collaborator

benoitf commented Aug 13, 2019

FYI @vitaliy-guliy the semantic checks are failing https://github.com/che-incubator/chectl#providing-pull-request

@vitaliy-guliy
Copy link
Contributor Author

FYI @vitaliy-guliy the semantic checks are failing https://github.com/che-incubator/chectl#providing-pull-request

I don't understand why. Each commit has signed-off part.

devfile.yaml Outdated
- name: chectl
source:
type: git
location: 'https://github.com/che-incubator/chectl.git'
location: 'https://github.com/vitaliy-guliy/chectl.git'
Copy link
Collaborator

Choose a reason for hiding this comment

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

will be reverted before merging ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted

@benoitf
Copy link
Collaborator

benoitf commented Aug 14, 2019

@vitaliy-guliy semantic commits check is not Signed commits
https://conventionalcommits.org

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
CONTRIBUTING.md Outdated

#### 'theia-dev' container

This container is used only for running Theia editor and does not take part in the following flow.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure to understand why we have a container that is not used ?
AFAIK we don't have theia-dev there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

theia-de, not theia-dev. It's my wrong.

CONTRIBUTING.md Outdated

## Package binaries

For packaging binaries we use https://github.com/oclif/dev-cli. It generates packaged for Linux, Windows and MacOS operation systems and puts the result in `dist/channels/stable` directory.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
For packaging binaries we use https://github.com/oclif/dev-cli. It generates packaged for Linux, Windows and MacOS operation systems and puts the result in `dist/channels/stable` directory.
For packaging binaries, [oclif](https://github.com/oclif/dev-cli) is used. It generates packages for Linux, Windows and MacOS operation systems and puts the result in `dist/channels/stable` directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@benoitf benoitf added this to the 7.0.0 milestone Aug 14, 2019
@benoitf benoitf merged commit d78ef53 into che-incubator:master Aug 14, 2019
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