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

Create PULL_REQUEST_TEMPLATE.md [skip ci] #550

Closed
wants to merge 6 commits into from

Conversation

183amir
Copy link
Contributor

@183amir 183amir commented May 4, 2016

@conda-forge/core I create a pull request template. What do you think?

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but couldn't find any.
Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@183amir
Copy link
Contributor Author

183amir commented May 4, 2016

@ocefpaf I opened a new one here.

@ocefpaf
Copy link
Member

ocefpaf commented May 4, 2016

@ocefpaf I opened a new one here.

No problems. Please see me comments on #549, they are minor easy to fix.

@183amir I believe we can put the template files (PR and Issues) in a .github directory to avoid the clutter.

@183amir 183amir changed the title Create PULL_REQUEST_TEMPLATE.md Create PULL_REQUEST_TEMPLATE.md [skip ci] May 4, 2016
@jakirkham
Copy link
Member

Nice idea. Will try to review later.

I believe one can use CONTRIBUTING.md and GitHub will link it to the top of every issue and PR.

@183amir
Copy link
Contributor Author

183amir commented May 4, 2016

True. There is also ISSUE_TEMPLATE.md

@ocefpaf
Copy link
Member

ocefpaf commented May 4, 2016

I would not worry about CONTRIBUTING.md and ISSUE_TEMPLATE.md right now. However, this PR template you put together is very useful and long overdue! Let's get that on the repo ASAP.

@@ -0,0 +1,37 @@
Recipes added with this pull request (max 5):
Copy link
Member

@jakirkham jakirkham May 5, 2016

Choose a reason for hiding this comment

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

Not sure that we should have a hard limit on this. For some people, dumping everything in one go is the easiest way to get started. Admittedly, we may end up breaking it up in the end, but it would be better for reviewers and contributors to work together to strike a healthy balance for this on a case-by-case basis.

Maybe some wording can be added to encourage a small number of recipes.

@jakirkham
Copy link
Member

Took a look. Largely in favor of most of it. Some things need tweaks. Here are a couple big points.

  • Python install: we should stick with the status quo until we make a decision on this ( Python install line standard #528 ).
  • Compilers: same story (though admittedly that changed very recently).
  • Example recipe: should update to be a better representation of what we do now.

@jakirkham
Copy link
Member

I believe we can put the template files (PR and Issues) in a .github directory to avoid the clutter.

👍 Correct. See ref.

@183amir
Copy link
Contributor Author

183amir commented May 5, 2016

I will be out till Monday. Please feel free to open another pull request based on this one and add your comments directly there if you want to merge this as soon as possible.

- nose

commands:
- nosetests -sv package
Copy link
Member

@jakirkham jakirkham May 16, 2016

Choose a reason for hiding this comment

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

IMHO the point of testing is not really to run CI for the packages that are given to us. Instead it is here to guarantee the following things.

  1. The compiler didn't mess something up.
  2. Something didn't go wrong in the linking stage.
  3. That the package will work in the environment defined.

The first two don't really apply to pure Python packages so let's ignore that for now. As far as solving the last one, importing modules seems to be sufficient for the most part. Running the full test suite is something the developer(s) hopefully are doing on releases at least if not during development as part of CI. Thus, there isn't really much value in us doing it to for pure Python packages. If the Python code does have compiled portions, that is a totally different story.

In the end, there may be a few packages that need something like run_test.py to do a few more things, but that will probably be determined after it is a feedstock.

In short, I don't think this should be a hard requirement for pure Python packages.

@183amir
Copy link
Contributor Author

183amir commented May 16, 2016

@jakirkham since you have written the guidelines and this is really urgent for people adding adding new recipes, why don't you create another pull request based on this one and add your comments directly to it? so that we can have this merged as fast as possible. We can always iterate this later.

@ocefpaf
Copy link
Member

ocefpaf commented Jul 29, 2018

Closing as "stale."

@ocefpaf ocefpaf closed this Jul 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants