Updating CONTRIBUTING.md for eclipse/che#21363 #2239

SDawley · 2022-09-01T20:05:58Z

What does this PR do?

Removed Generate Devfile and Create Workspace sections and edited the command documentation to reflect how commands are currently run.

Signed-off-by: Samantha Dawley [email protected]

Screenshot/screencast of this PR

What issues does this PR fix or reference?

Addresses: eclipse-che/che#21363

How to test this PR?

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

openshift-ci · 2022-09-01T20:06:10Z

Hi @SDawley. Thanks for your PR.

I'm waiting for a che-incubator member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tolusha · 2022-09-02T08:40:31Z

Sounds good. What about putting sandbox url instead of <your-che-host> to have defined url to start a workspace ?

SDawley · 2022-09-02T21:10:34Z

Sounds good. What about putting sandbox url instead of <your-che-host> to have defined url to start a workspace ?

OK, switched to the OpenShift Sandbox url.

max-cx

Just a quick, cursory review.

CONTRIBUTING.md

SDawley · 2022-09-08T14:21:18Z

Updated including Max, Florent, and Nick's suggested fixes.

tolusha · 2022-09-08T14:39:04Z

Remove Create Workspace and Generate Devfile task from the Devfile.
I created a workspace on dogfooding instance https://che-dogfooding.apps.che-dev.x6e0.p1.openshiftapps.com/f?url=https://github.com/che-incubator/chectl with che-code as a default editor. After running che: build task I got:

yarn install v1.22.17
[1/5] Validating package.json...
error [email protected]: The engine "node" is incompatible with this module. Expected version ">=16.0.0". Got "14.19.0"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

nickboldt · 2022-09-08T16:47:09Z

Remove Create Workspace and Generate Devfile task from the Devfile.

I created a workspace on dogfooding instance https://che-dogfooding.apps.che-dev.x6e0.p1.openshiftapps.com/f?url=https://github.com/che-incubator/chectl with che-code as a default editor. After running che: build task I got:
yarn install v1.22.17
[1/5] Validating package.json...
error [email protected]: The engine "node" is incompatible with this module. Expected version ">=16.0.0". Got "14.19.0"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

Strange. The Che UDI image has 3 different vesions of node (12, 14, and 16) so you should be able to build it there using that image:

https://github.com/devfile/developer-images/blob/main/universal/ubi8/Dockerfile#L62-L71

SDawley · 2022-09-13T22:54:35Z

I created a workspace and manually updated the image to 'quay.io/devfile/universal-developer-image:ubi8-latest'

The build command now works, as does run but package-binaries is failing so I am investigating.

tolusha · 2022-09-23T07:58:10Z

@SDawley
Pls, convert devfile from apiVersion: 1.0.0 to apiVersion: 2.1.0.

nickboldt

Tests are passing so if @tolusha is happy, then +1 to merge.

SDawley · 2022-09-26T22:53:50Z

@SDawley Pls, convert devfile from apiVersion: 1.0.0 to apiVersion: 2.1.0.

@tolusha
I've updated the devfile to 2.1.0 and added an extensions.json with redhat.vscode-yaml, I did not find che-incubator.typescript in open-vsx.org.

I'm able to create a factory link from this branch, but package-binaries still fails with:
/bin/sh: shasum: command not found Error: Command failed: /bin/sh -c grep node-v16.14.0-linux-x64.tar.xz /projects/chectl/tmp/cache/16.14.0/SHASUMS256.txt.asc | shasum -a 256 -c - Code: 127 error Command failed with exit code 1.
I'm fairly certain its oclif that's requiring shasum. So we can either find a new packaging method or add a symlink to the container from sha265sum to shasum.

Everything else seems to be working, though the tests take a lot longer to run in the workspace than they do locally

tolusha · 2022-09-27T06:30:42Z

I did not find che-incubator.typescript

Have a look. I think you can add both.
https://open-vsx.org/extension/vscode/typescript-language-features
https://open-vsx.org/extension/vscode/typescript

tolusha · 2022-09-27T11:45:12Z

Remove projects: from the devfile, otherwise it always clone the main branch from the che-incubator/chectl` repository

tolusha · 2022-09-27T11:46:09Z

Use '${PROJECT_SOURCE}' as workingDir

tolusha · 2022-09-27T11:57:47Z

Have a look at https://github.com/che-samples/react-web-app/blob/devfilev2/devfile.yaml
Pls add label and group to the commands.

tolusha · 2022-09-27T12:19:25Z

Use yarn lint:fix as format source command

.vscode/launch.json

devfile.yaml

package.json

tolusha · 2022-09-28T09:02:39Z

package.json

-    "jest": "^26.6.3",
-    "ts-jest": "^26.5.6",
+    "jest": "^29.0.3",
+    "ts-jest": "^29.0.1",


Why do we need this?

The jest tests were failing in the workspace at version 26 and updating to 29 fixed it. Enough else has changed that I can try reverting that and see if the issue comes back.

I tried and did not face any issue with 26.x versions. Please revert and check.

tolusha · 2022-09-28T09:03:41Z

So we can either find a new packaging method or add a symlink to the container from sha265sum to shasum.

We definitely need to add shasum into a dev container
I spotted another problem: yarn test takes forever. To fix this issue, pls add for a dev container:

      cpuLimit: '2'
      cpuRequest: 500m
      memoryLimit: 5G
      memoryRequest: 1G

SDawley · 2022-09-28T23:52:54Z

So we can either find a new packaging method or add a symlink to the container from sha265sum to shasum.

We definitely need to add shasum into a dev container I spotted another problem: yarn test takes forever. To fix this issue, pls add for a dev container:
      cpuLimit: '2'
      cpuRequest: 500m
      memoryLimit: 5G
      memoryRequest: 1G

I tried adding this to the devfile and restarting the workspace in RHOSDS and got the error:
Detected unrecoverable deployment condition: FailedCreate pods "workspace8fb0fe516e274bf6-6fbd749ff6-89695" is forbidden: exceeded quota: for-sdawley-compute, requested: limits.memory=6042653184, used: limits.memory=6042653184, limited: limits.memory=7Gi

tolusha · 2022-09-29T07:30:04Z

I tried adding this to the devfile and restarting the workspace in RHOSDS and got the error:

There is no much we can do here. If it requires at least 5G memory, then we have to use another cluster.
I personally use https://che-dogfooding.apps.che-dev.x6e0.p1.openshiftapps.com/dashboard/

l0rd

@SDawley that's great work. I have added some comments with suggestions.

.vscode/extensions.json

l0rd · 2022-10-04T14:36:09Z

devfile.yaml

+  - name: dev
+    container: 
+      image: 'quay.io/devfile/universal-developer-image:ubi8-latest'
+      memoryLimit: 1G


I think we didn't support it at the time this devfile was introdcued but it's better to specify the cpuLimit too:

Suggested change

memoryLimit: 1G

memoryLimit: 1G

cpuLimit: 500m

devfile.yaml

CONTRIBUTING.md

dkwon17

Looks great, I was able to build, test and run chectl in my workspace. I just have some small suggestions

CONTRIBUTING.md

openshift-ci · 2022-10-10T14:52:25Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: l0rd, nickboldt, SDawley

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

SDawley · 2022-10-12T00:10:08Z

I had to rebuild the universal image because it didn't use the new base image, but packaging binaries now works.
@tolusha

tolusha · 2022-10-12T07:19:51Z

I have another error now related to yarn pack-binaries command

oclif-dev: building target chectl-win32-x64.tar.gz
/bin/sh: 7z: command not found
 ›   Error: install 7-zip to package windows tarball
error Command failed with exit code 2.

openshift-ci · 2022-10-20T21:13:49Z

New changes are detected. LGTM label has been removed.

SDawley · 2022-10-20T21:35:09Z

@tolusha
I've moved the commands from the devfile to the .vscode/tasks.json since that makes finding and executing them easier.
Package binaries is working now that 7zip is in ubi8.

tolusha · 2022-10-21T06:44:58Z

Sounds good!

tolusha · 2022-10-21T06:52:25Z

Pls, fix DCO issue.

Signed-off-by: Samantha Dawley <[email protected]>

SDawley requested review from tolusha and flacatus as code owners September 1, 2022 20:05

openshift-ci bot added the needs-ok-to-test label Sep 1, 2022

SDawley force-pushed the contribute branch from 1c530fe to bf951ab Compare September 2, 2022 21:12

max-cx reviewed Sep 5, 2022

View reviewed changes

nickboldt self-requested a review September 23, 2022 21:49

nickboldt approved these changes Sep 23, 2022

View reviewed changes

openshift-ci bot assigned nickboldt Sep 23, 2022

openshift-ci bot added lgtm and removed lgtm labels Sep 23, 2022

tolusha reviewed Sep 27, 2022

View reviewed changes

.vscode/launch.json Outdated Show resolved Hide resolved

tolusha reviewed Sep 28, 2022

View reviewed changes

devfile.yaml Outdated Show resolved Hide resolved

tolusha reviewed Sep 28, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

tolusha reviewed Sep 28, 2022

View reviewed changes

SDawley mentioned this pull request Sep 29, 2022

Feat: Small change to add shasum for chectl devfile/developer-images#35

Merged

l0rd reviewed Oct 4, 2022

View reviewed changes

dkwon17 reviewed Oct 4, 2022

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

dkwon17 reviewed Oct 4, 2022

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

dkwon17 reviewed Oct 4, 2022

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

dkwon17 reviewed Oct 4, 2022

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

SDawley mentioned this pull request Oct 5, 2022

Fix: actual install command is perl-Digest-SHA devfile/developer-images#37

Merged

l0rd approved these changes Oct 10, 2022

View reviewed changes

openshift-ci bot assigned l0rd Oct 10, 2022

openshift-ci bot added the lgtm label Oct 10, 2022

This was referenced Oct 12, 2022

fix: add p7zip-plugins to base image for chectl devfile/developer-images#39

Merged

Fix: add epel repo so p7zip can be found. devfile/developer-images#41

Merged

openshift-ci bot removed the lgtm label Oct 20, 2022

Fix: forgot to sign a commit

f921259

Signed-off-by: Samantha Dawley <[email protected]>

SDawley force-pushed the contribute branch from a44fc6d to f921259 Compare October 21, 2022 17:39

tolusha merged commit 30e845d into che-incubator:main Oct 24, 2022

che-bot added this to the 7.56 milestone Oct 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating CONTRIBUTING.md for eclipse/che#21363 #2239

Updating CONTRIBUTING.md for eclipse/che#21363 #2239

SDawley commented Sep 1, 2022 •

edited

Loading

openshift-ci bot commented Sep 1, 2022

tolusha commented Sep 2, 2022 •

edited

Loading

SDawley commented Sep 2, 2022

max-cx left a comment •

edited

Loading

SDawley commented Sep 8, 2022

tolusha commented Sep 8, 2022

nickboldt commented Sep 8, 2022

SDawley commented Sep 13, 2022

tolusha commented Sep 23, 2022

nickboldt left a comment

SDawley commented Sep 26, 2022

tolusha commented Sep 27, 2022 •

edited

Loading

tolusha commented Sep 27, 2022

tolusha commented Sep 27, 2022

tolusha commented Sep 27, 2022

tolusha commented Sep 27, 2022

tolusha Sep 28, 2022

SDawley Sep 28, 2022

tolusha Sep 29, 2022 •

edited

Loading

tolusha commented Sep 28, 2022 •

edited

Loading

SDawley commented Sep 28, 2022

tolusha commented Sep 29, 2022

l0rd left a comment

l0rd Oct 4, 2022

dkwon17 left a comment

openshift-ci bot commented Oct 10, 2022

SDawley commented Oct 12, 2022

tolusha commented Oct 12, 2022 •

edited

Loading

openshift-ci bot commented Oct 20, 2022

SDawley commented Oct 20, 2022

tolusha commented Oct 21, 2022 •

edited

Loading

tolusha commented Oct 21, 2022

Updating CONTRIBUTING.md for eclipse/che#21363 #2239

Updating CONTRIBUTING.md for eclipse/che#21363 #2239

Conversation

SDawley commented Sep 1, 2022 • edited Loading

What does this PR do?

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Reviewers

openshift-ci bot commented Sep 1, 2022

tolusha commented Sep 2, 2022 • edited Loading

SDawley commented Sep 2, 2022

max-cx left a comment • edited Loading

Choose a reason for hiding this comment

SDawley commented Sep 8, 2022

tolusha commented Sep 8, 2022

nickboldt commented Sep 8, 2022

SDawley commented Sep 13, 2022

tolusha commented Sep 23, 2022

nickboldt left a comment

Choose a reason for hiding this comment

SDawley commented Sep 26, 2022

tolusha commented Sep 27, 2022 • edited Loading

tolusha commented Sep 27, 2022

tolusha commented Sep 27, 2022

tolusha commented Sep 27, 2022

tolusha commented Sep 27, 2022

tolusha Sep 28, 2022

Choose a reason for hiding this comment

SDawley Sep 28, 2022

Choose a reason for hiding this comment

tolusha Sep 29, 2022 • edited Loading

Choose a reason for hiding this comment

tolusha commented Sep 28, 2022 • edited Loading

SDawley commented Sep 28, 2022

tolusha commented Sep 29, 2022

l0rd left a comment

Choose a reason for hiding this comment

l0rd Oct 4, 2022

Choose a reason for hiding this comment

dkwon17 left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Oct 10, 2022

SDawley commented Oct 12, 2022

tolusha commented Oct 12, 2022 • edited Loading

openshift-ci bot commented Oct 20, 2022

SDawley commented Oct 20, 2022

tolusha commented Oct 21, 2022 • edited Loading

tolusha commented Oct 21, 2022

SDawley commented Sep 1, 2022 •

edited

Loading

tolusha commented Sep 2, 2022 •

edited

Loading

max-cx left a comment •

edited

Loading

tolusha commented Sep 27, 2022 •

edited

Loading

tolusha Sep 29, 2022 •

edited

Loading

tolusha commented Sep 28, 2022 •

edited

Loading

tolusha commented Oct 12, 2022 •

edited

Loading

tolusha commented Oct 21, 2022 •

edited

Loading