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

simplify code contributions + reviews by fully automating the dev setup. #1226

Merged
merged 1 commit into from
May 4, 2020
Merged

simplify code contributions + reviews by fully automating the dev setup. #1226

merged 1 commit into from
May 4, 2020

Conversation

nisarhassan12
Copy link
Contributor

Hi 🙂

This Pr simplifies code contributions by fully automating the dev setup with Gitpod(a full-featured online cloud IDE) which is free for OSS. With a single click, it'll launch a workspace and automatically:

  • clone the dvc.org repo.
  • install the dependencies.
  • run yarn build.
  • start the webserver via yarn start.

so that anyone interested in contributing can start straight away without any friction.

this is how it looks:

image

You can give it a try on my fork of the repo via the following link:

https://gitpod.io/#https://github.com/nisarhassan12/dvc.org

A lot of popular OSS projects out there are already using Gitpod to streamline the experience for their contributors and maintainers, some of them are:

You can find a brief list of them at https://contribute.dev

image

Prs can also be reviewed from within Gitpod.

@shcheklein
Copy link
Member

@nisarhassan12 looks cool!

@casperdcl link check fails again, looks like a false positive.

@casperdcl
Copy link
Contributor

much sneaky such wow. This pattern:

[](https://gitpod.io/#https://github.com/iterative/dvc.org)

is pretty darn hard to fix. I'd recommend just adding it to the excludes rather than coming up with a solution. Currently the above line results in:

https://github.com/iterative/dvc.org)
https://gitpod.io/#https://github.com/iterative/dvc.org

so https://github.com/iterative/dvc.org) would have to be added to scripts/exclude-links.txt

@shcheklein
Copy link
Member

why does it result in https://github.com/iterative/dvc.org) ... don't we ignore )?

@casperdcl
Copy link
Contributor

nope because parentheses are valid chars in URLs. There are many in the docs, e.g. https://en.wikipedia.org/wiki/Hyperparameter_(machine_learning)

@casperdcl
Copy link
Contributor

I'll suggest a minor fix in a PR but would be best to double-check...

@shcheklein
Copy link
Member

shcheklein commented May 4, 2020

@nisarhassan12 could you please rebase against or merge the latest master? to fix CI check

.gitpod.yml Show resolved Hide resolved
@@ -51,6 +51,14 @@ Otherwise, please refer to the following procedure:

We will review your PR as soon as possible. Thank you for contributing!

## Automated development environment in a single click
Copy link
Member

Choose a reason for hiding this comment

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

let's make it a paragraph in the Dev environment. Like a second paragraph for example should be a good fit. May remove "locally" from the first paragraph.

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 have made the change.

@nisarhassan12
Copy link
Contributor Author

@nisarhassan12 could you please rebase against or merge the latest master? to fix CI check

Thanks! @shcheklein I have rebased it againist the latest master.

@shcheklein
Copy link
Member

@nisarhassan12 awesome, one more ask - please check the comment. I think we can make it part of the dev env section in the docs.

@shcheklein shcheklein merged commit 6898bb9 into iterative:master May 4, 2020
@shcheklein
Copy link
Member

done, merged! and I moved a bit in the master ... thanks 🙏 Hopefully we'll be getting even more contributors now. Will you make a PR to the list of sites, btw?

@nisarhassan12
Copy link
Contributor Author

@shcheklein sure! please open an issue here https://github.com/gitpod-io/contribute.dev/ 🙂

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented May 5, 2020

Sorry for the delay. This is really cool 😄

The button looks a little strange in https://dvc.org/doc/user-guide/contributing/docs#development-environment though:

image

I can address the formatting in a future PR, but the button is not working for me:

image

Can you maybe check what the problem is, @nisarhassan12? It works from https://contribute.dev/

@shcheklein
Copy link
Member

Yep, does not work for me either (worked yesterday).

Also, we need to adjust CSS a bit to remove the "external link" icon. @jorgeorpinel mind creating a ticket for this?

@nisarhassan12
Copy link
Contributor Author

Can you maybe check what the problem is, @nisarhassan12? It works from https://contribute.dev/

Sure. I will shortly make a Pr with the fix.

@jorgeorpinel

This comment has been minimized.

@jorgeorpinel
Copy link
Contributor

Hmmm I tried using Gitpod to format the note I mentioned above. It was all working nicely, I even installed a VS Code plugin to the IDE. But after committing with Git, I'm unable to push because I didn't grant Gitpod permission to push directly to dvc.org

image

Open source contributors will never have this permission. Can it be made so a fork is created in their accounts first, and then Gitpod launched from there? Or maybe if I was using a different user, that would've been the behavior... Not sure

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented May 6, 2020

Or maybe if I was using a different user, that would've been the behavior... Not sure

Nope. I tried with @AyokVoluntarios which I also own and it tries to use the upstream directly:

image

So even if I tell Github to let Gitpod push to my repos, I simply can't push to iterative/dvc.org from that account.

image

The user would need to configure the Git remote and branch manually from the IDE. Do you know whether that's the only solution, @nisarhassan12? Thanks

Cc @shcheklein

@shcheklein
Copy link
Member

@jorgeorpinel good catch, let's create a separate ticket and ping @nisarhassan12 there?

@jorgeorpinel
Copy link
Contributor

Sure, done: #1253

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.

4 participants