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 codespaces template for creating new repo's. #1957

Merged
merged 26 commits into from
Nov 3, 2022
Merged

Conversation

vsmalladi
Copy link
Contributor

@vsmalladi vsmalladi commented Oct 18, 2022

PR checklist

Add codespaces template for when creating a new repository. Closes #1953

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated

@vsmalladi vsmalladi requested review from ggabernet and ewels October 18, 2022 21:18
@codecov
Copy link

codecov bot commented Oct 18, 2022

Codecov Report

Merging #1957 (0146a89) into dev (7848faa) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev    #1957   +/-   ##
=======================================
  Coverage   62.50%   62.50%           
=======================================
  Files          43       43           
  Lines        4902     4902           
=======================================
  Hits         3064     3064           
  Misses       1838     1838           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@vsmalladi vsmalladi requested a review from ewels October 19, 2022 16:02
Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

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

Super nice! Excited to be able to play with this 😀

Any chance we could also add something similar to the top-level of the repo to also have it available to work on nf-core/tools? (in addition to pipelines)

nf_core/pipeline-template/.devcontainer/Dockerfile Outdated Show resolved Hide resolved
nf_core/pipeline-template/.devcontainer/devcontainer.json Outdated Show resolved Hide resolved
nf_core/pipeline-template/.devcontainer/devcontainer.json Outdated Show resolved Hide resolved
nf_core/pipeline-template/.devcontainer/Dockerfile Outdated Show resolved Hide resolved
nf_core/pipeline-template/.devcontainer/noop.txt Outdated Show resolved Hide resolved
nf_core/pipeline-template/README.md Outdated Show resolved Hide resolved
nf_core/pipeline-template/README.md Outdated Show resolved Hide resolved
@mahesh-panchal
Copy link
Member

mahesh-panchal commented Oct 28, 2022

I did more digging around and there should be some way to combine these: https://github.com/gitpod-io/template-gitpod-sxs-codespaces. The Codespaces environment is using the Gitpod image here.

The Gitpod base image does have a lot of things we generally don't need, but it was used for docker and java included ( full list is https://github.com/gitpod-io/workspace-images/blob/HEAD/dazzle.yaml#L3. I was wondering initially if the Gitpod image could use the devcontainers image as a base, but I can't find what's included in them.

Also what's the difference between Codespaces and Gitpod ?
Gitpod has it's own comparison https://www.gitpod.io/vs/github-codespaces. but I haven't found a simple independent review

@vsmalladi
Copy link
Contributor Author

@mahesh-panchal and @ewels got the gitpod to work with codespaces to use the same base image. Give it a test.

@ewels
Copy link
Member

ewels commented Oct 28, 2022

@nf-core-bot fix linting

Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

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

Looking great! Much leaner 🚀

@mahesh-panchal - what do you think about moving the GitPod files into .devcontainer so that everything is together? It shouldn't matter where those files are, right?

.devcontainer/devcontainer.json Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Show resolved Hide resolved
@mahesh-panchal
Copy link
Member

what do you think about moving the GitPod files into .devcontainer so that everything is together? It shouldn't matter where those files are, right?

The .gitpod.yml's need to be where they are. What else is there to move?

@vsmalladi
Copy link
Contributor Author

I think we could do a symbolic link if you want them in the same place. But that would the best solution to deal with harcoded locations these programs are looking for information.

@mahesh-panchal
Copy link
Member

I think just keep the files where they are. Symlinking for the sake of grouping creates another file, and potential confusion when looking to update those files.

@ewels
Copy link
Member

ewels commented Nov 2, 2022

Ok, sounds good! Just my final outstanding 3 comments @vsmalladi then can merge 👍🏻

@vsmalladi
Copy link
Contributor Author

@ewels awesome fixed outstanding issues. Should be good.

Copy link
Member

@mahesh-panchal mahesh-panchal left a comment

Choose a reason for hiding this comment

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

The failing tests don't seem related to the files here so I'm going to approve it.

@mashehu
Copy link
Contributor

mashehu commented Nov 3, 2022

@nf-core-bot fix linting

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

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

Was only minor tweaks left, so I fixed up myself. LGTM! Thanks! 🚀

@ewels ewels merged commit 370efaa into nf-core:dev Nov 3, 2022
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.

5 participants