HEP-FCC / heppy

[deprecated] A python analysis framework for high energy physics
Other
11 stars 32 forks source link

Renaming heppy executables #89

Closed vvolkl closed 5 years ago

vvolkl commented 5 years ago

This might require updating a few scripts in some places, but I believe it is worthwhile. When heppy is installed as a package, it means that dataset.py and other scripts are put into $PATH. Without the context of the repo this will most likely lead to confusion, so I propose to prefix everything with heppy_. To make up for the longer names, the .py suffixes are removed.

cbernet commented 5 years ago

Hi Valentin, This repo is > 30 commits behind mine. In particular, I restructured heppy to make it possible to install through pypi. I think it would be better maybe that I first do a PR, and then that we include yours. What do you think?

JavierCVilla commented 5 years ago

I think it would be better maybe that I first do a PR, and then that we include yours

I like the idea of aligning both repos, indeed it would be nice to have this repo up to date more often. Since I know you like to have all your developments in a different fork, I would propose to create a PR as soon as you come up with a new feature in your repo.

cbernet commented 5 years ago

Hi Javier,

I’m a bit lost :-)

I was saying that we should merge a PR of mine before the one of Valentin, because there are many changes on my fork, affecting the whole structure of the heppy package. You’re saying that you also want to do a PR to heppy, and that it should come first. Before mine? Before the one of Valentin? What is your PR and why would you like it to come first?

In the meanwhile I’ll send mine. We can decide later what we merge first.

Cheers,

Colin

On 28 Feb 2019, at 10:49, Javier Cervantes notifications@github.com<mailto:notifications@github.com> wrote:

I think it would be better maybe that I first do a PR, and then that we include yours

I like the idea of aligning both repos, indeed it would be nice to have this repo up to date more often. Since I know you like to have all your developments in a different fork, I would propose to create a PR as soon as you come up with a new feature in your repo.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHubhttps://github.com/HEP-FCC/heppy/pull/89#issuecomment-468207874, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AD8ku1Rd_gMK74ljaC4Hm0EJbXQLlY6mks5vR6YigaJpZM4bVn4t.

JavierCVilla commented 5 years ago

Hi Colin,

Sorry, maybe I did not explain myself.

I'm not proposing any PR. I'm just sharing my opinion about the current policy that we are following.

because there are many changes on my fork, affecting the whole structure of the heppy package.

In my opinion, this should not be the case. We should have all changes merged into this repository as soon as they are ready, so that we avoid having many changes in one external place (your fork) that suddenly need to be merged at the same time.

cbernet commented 5 years ago

Ah now I understand what’s going on. When you quote text, your mailer only inserts a tab, so your quoted text is not really visible.

Colin

On 28 Feb 2019, at 10:58, Javier Cervantes notifications@github.com<mailto:notifications@github.com> wrote:

Hi Colin,

Sorry, maybe I did not explain myself.

I'm not proposing any PR. I'm just sharing my opinion about the current policy that we are following.

because there are many changes on my fork, affecting the whole structure of the heppy package.

In my opinion, this should not be the case. We should have all changes merged into this repository as soon as they are ready, so that we avoid having many changes in one external place (your fork) that need to be merged at the same time.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHubhttps://github.com/HEP-FCC/heppy/pull/89#issuecomment-468210995, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AD8ku2kca1Tc2vxwiHEqths4lNboQBXkks5vR6hdgaJpZM4bVn4t.

vvolkl commented 5 years ago

@phsft-bot build

vvolkl commented 5 years ago

test this please

phsft-bot commented 5 years ago

Build or tests failed.

phsft-bot commented 5 years ago

All tests succeeded.