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

Added CONTRIBUTING.md and edited README.md file with instructions to install and run the project #13

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

Conversation

IvaniGabrovsky
Copy link

I added instructions in the README.md file to install dependancies and run the project. I added instructions how to contrbute to this project in the CONTRIBUTING.md file.

Copy link

@SerpentBytes SerpentBytes left a comment

Choose a reason for hiding this comment

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

Is it possible to use only markdown syntax in the .md file? Can we replace <br> with the markdown equivalent of br tag?

@IvaniGabrovsky
Copy link
Author

Is it possible to use only markdown syntax in the .md file? Can we replace <br> with the markdown equivalent of br tag?

I am not sure what is the equilevant in .md to
?

@SerpentBytes
Copy link

@IvaniGabrovsky, actually it's fine. LGTM.

README.md Outdated

## Usage

To see the application running type F5. A new window will pop up with a hello message.
Copy link
Contributor

Choose a reason for hiding this comment

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

Type F5 in what? I think this is missing a bunch of info and needs more detail

Copy link
Author

Choose a reason for hiding this comment

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

I added more detail. Please take a look at it.

Copy link

@SerpentBytes SerpentBytes left a comment

Choose a reason for hiding this comment

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

I am viewing the document on GitHub here

I think in some places, we don't really need <br> tags, as it's adding unnecessary padding. For instance, after the first heading, and before the first instance of the bolded heading.

@SerpentBytes
Copy link

Also, I don't think you need to make the heading bold, because it wouldn't do anything.

CONTRIBUTING.md Outdated
@@ -0,0 +1,86 @@
# Guidelines for Contribution

<br>

Choose a reason for hiding this comment

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

This is adding extra padding.

CONTRIBUTING.md Outdated

<br>

## **Here is a brief overview of how to contribute to vscode-seneca-college:**

Choose a reason for hiding this comment

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

You don't need to make Headings bold. They are by default bolded.

CONTRIBUTING.md Outdated

14. After your pull request is merged, celebrate your achievement!

## <br>

Choose a reason for hiding this comment

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

This would render in HTML: <h2 id=""><br></h2> I don't think you need to make it a heading.

CONTRIBUTING.md Outdated

## <br>

### THANK YOU FOR YOUR PARTICIPATION!

Choose a reason for hiding this comment

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

I think you shouldn't conclude a document with a heading.

README.md Outdated

## Usage

To see the application running type F5 with your keyboard in any file of the project. Just make sure your mouse is in any of files in the project. A new window will pop up with a hello message. You are now in debugging mode. You can debug the code to find out what it is doing with the step forward step inward step out of buttons. Or press the red square to exit debugging mode.
Copy link
Contributor

Choose a reason for hiding this comment

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

This still doesn't make sense to me.

Imagine I do exactly what you just said:

  1. I run npm i to install the dependencies (I'm in a terminal)
  2. Now I cd into the project folder. This seems odd, since the project folder should be where I do npm i
  3. Now I press F5 in my terminal? That doesn't do anything

Choose a reason for hiding this comment

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

Checking out package.json scripts is probably a good place to start:

"scripts": {
"build": "tsup src/index.ts --external vscode",
"dev": "nr build --watch",
"lint": "eslint .",
"vscode:prepublish": "nr build",
"publish": "vsce publish --no-dependencies",
"pack": "vsce package --no-dependencies",
"test": "vitest",
"typecheck": "tsc --noEmit",
"release": "bumpp && nr publish"
},

Copy link
Author

Choose a reason for hiding this comment

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

I fixed it. Let me know if it is better.

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

Also, agree with @SerpentBytes re: removing the <br> tags (not necessary) and also letting Markdown properly format the titles (e.g., you don't need to bold them).

Copy link

@gulyapulya gulyapulya left a comment

Choose a reason for hiding this comment

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

Some suggestions:

CONTRIBUTING.md Outdated

<br>

## How can I contribute in vscode-seneca-college project?

Choose a reason for hiding this comment

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

I think grammatically, it would be better to say "contribute to" rather than "contribute in"

CONTRIBUTING.md Outdated

<br>

Never contributed to open source before? Are you curious in how contributions function in our project?

Choose a reason for hiding this comment

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

Suggestion: "curious about"

CONTRIBUTING.md Outdated

<br>

1. Start by identifying an issue or a feature that you would want to add.

Choose a reason for hiding this comment

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

Probably good to mention that contributors can file issues themselves if they have an idea that has not been documented yet or choose an existing issue from the "issues" tab.

CONTRIBUTING.md Outdated

<br>

2. In your local GitHub organization, fork the repository linked to the problem. The repository `underyour-GitHub-username/repository-name` will now be accessible to you.

Choose a reason for hiding this comment

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

Most probably a personal user account will be used rather than an organization, so can be changed to "Fork current repository to produce a personal copy of this project by clicking the "Fork" button in the upper right corner of your screen. If you leave the default settings, the repository "your-username/vscode-seneca-college" will now be appear under your profile."

README.md Outdated

## Usage

To see the application running type F5 with your keyboard in any file of the project. Just make sure your mouse is in any of files in the project. A new window will pop up with a hello message. You are now in debugging mode. You can debug the code to find out what it is doing with the step forward step inward step out of buttons. Or press the red square to exit debugging mode.

Choose a reason for hiding this comment

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

Checking out package.json scripts is probably a good place to start:

"scripts": {
"build": "tsup src/index.ts --external vscode",
"dev": "nr build --watch",
"lint": "eslint .",
"vscode:prepublish": "nr build",
"publish": "vsce publish --no-dependencies",
"pack": "vsce package --no-dependencies",
"test": "vitest",
"typecheck": "tsc --noEmit",
"release": "bumpp && nr publish"
},

@IvaniGabrovsky
Copy link
Author

I just did all the feedback that gulyapulya suggested me to do. Please let me know if it is better.

@gulyapulya
Copy link

Good job @IvaniGabrovsky! CONTRIBUTING.md seems good (@SerpentBytes suggestions were also nice to have but not necessary). I think it can be merged but the README.md needs more work.

@IvaniGabrovsky
Copy link
Author

Good job @IvaniGabrovsky! CONTRIBUTING.md seems good (@SerpentBytes suggestions were also nice to have but not necessary). I think it can be merged but the README.md needs more work.

I don't see what else needs more work in README.md. Could you be more descriptive to what you think needs to be added in README.md

Copy link

@rudychung rudychung left a comment

Choose a reason for hiding this comment

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

Since there are so many steps to contribution, you may want to separate them under different headings. Maybe something like "Installing Locally", "Submitting Your Changes", etc., or you could condense some of your steps together?


## **Here is a brief overview of how to contribute to vscode-seneca-college:**

---

Choose a reason for hiding this comment

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

I don't think that this horizontal rule is necessary, since the header above already separates the content above from the content below.

CONTRIBUTING.md Outdated

<br>

2. In your local GitHub organization, Fork current repository to produce a personal copy of this project by clicking the "Fork" button in the upper right corner of your screen. If you leave the default settings, the repository `underyour-GitHub-username/repository-name` will now be appear under your profile.

Choose a reason for hiding this comment

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

Minor grammatical issue.
"will now be appear under your profile." should be "will now appear under your profile.". Although, a better change might be "should appear under your profile.", since the default settings might not always have the repo appear under your profile (e.g., if you've forked the repo already, it might default to forking it under an organization you're a part of).

Copy link
Author

Choose a reason for hiding this comment

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

I fixed the gramatic error.

@IvaniGabrovsky
Copy link
Author

I applied all of your changes. Let me know if it is better now.

Copy link

@SerpentBytes SerpentBytes left a comment

Choose a reason for hiding this comment

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

The changes I requested looks good to me. Good job.

@gulyapulya
Copy link

Good job @IvaniGabrovsky! CONTRIBUTING.md seems good (@SerpentBytes suggestions were also nice to have but not necessary). I think it can be merged but the README.md needs more work.

I don't see what else needs more work in README.md. Could you be more descriptive to what you think needs to be added in README.md

I think the professor's comments about the installation steps in README.md are valid, they are mostly missing, but we can leave it for now as is and add details later. One step to add from the top of my mind is that after the installation to run the extension in development mode, the developer can go to the run and debug tab on the left in VSCode ( shft+cmd+D) and click run extension. It will open a new window with the vscode in dev mode.

README.md Outdated

## Usage

Make sure your mouse is in any project files, not in the terminal but on any line of code. Then to see the application running. Type F5 with your keyboard. A new window will pop up with a hello message. You are now in debugging mode. You can debug the code to find out what it is doing with the step forward, step inward, and step out of buttons. Or press the red square to exit debugging mode.

Choose a reason for hiding this comment

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

I believe this line is confusing Make sure your mouse is in any project files, not in the terminal but on any line of code.

You can simply say Make sure your Text cursor is in Editor

Copy link
Author

Choose a reason for hiding this comment

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

I edited it.


---

1. Start by identifying an issue or a feature that you would want to add. Choose a issue from the issues tab that you would like to work on. Feel free to open a new issue in the project if it is not in the list of issues.

Choose a reason for hiding this comment

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

If I am being REALLY nit picky, the second sentence should also be an issue instead of a issue.

git clone https://github.com/github-username/vscode-seneca-college.
```

4. Create working branch:
Copy link
Contributor

@Myrfion Myrfion Nov 25, 2022

Choose a reason for hiding this comment

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

Don't wanna be super picky, but you should probably add a between the working branch so -> 4. Create a working branch:

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.

8 participants