cms-gem-daq-project / gem-plotting-tools

Repository for GEM commissioning plotting tools
GNU General Public License v3.0
1 stars 26 forks source link

Update P5 setup instructions #121

Closed lmoureaux closed 6 years ago

lmoureaux commented 6 years ago

Description

The setup instructions for P5 were broken by updates in the system. gem-plotting-tools is now installed system-wide and a setup script is provided centrally.

Types of changes

Motivation and Context

Setup instructions for P5 are broken. See eg the discussion after https://mattermost.web.cern.ch/cms-gem-ops/pl/4on3wpg5jpboff1cwjzjfmxd3e

How Has This Been Tested?

Checklist:

(Sorry about the branch name, it was created automatically by Github and I don't know how to change it)

bdorney commented 6 years ago

Was there an issue in setting up a venv at P5? If so how was that resolved?

Moreover instructions of this nature should not be removed. But segmented into developer and user categories. Return the previous venv instructions (if they work) and class them as a developer instructions.

Add a statement that if someone doesn't know if they are a developer or user category that they fall into the user category.

lmoureaux commented 6 years ago

Was there an issue in setting up a venv at P5? If so how was that resolved?

Yes. It is resolved by telling the user to use the system-wide install.

Moreover instructions of this nature should not be removed. But segmented into developer and user categories. Return the previous venv instructions (if they work) and class them as a developer instructions.

They don't work anymore, and @jsturdy isn't interested in supporting non-system installs at P5 unless there's a good use-case (which I couldn't find in the Mattermost discussion linked to in the summary)

bdorney commented 6 years ago

Yes. It is resolved by telling the user to use the system-wide install.

Yes that's fine for a user; but preserve the instructions for a developer.

They don't work anymore, and @jsturdy isn't interested in supporting non-system installs at P5 unless there's a good use-case (which I couldn't find in the Mattermost discussion linked to in the summary)

Your mattermost link refers to a lack of virtualenv and pip but immediately below that link @mexanick and I both reference the cc7 virtual machine which does have the packages required as illustrated here.

bdorney commented 6 years ago

Is this PR stale?

lmoureaux commented 6 years ago

This issue is not stale, he issue you raised is being addressed in #125.

bdorney commented 6 years ago

This issue is not stale, he issue you raised is being addressed in #125.

Given you have #125, and it needs to be made even with central, I would just include these changes there if you have not already done so.

lmoureaux commented 6 years ago

I would just include these changes there if you have not already done so.

It wouldn't fit the topic of #125, "Developer instructions"

bdorney commented 6 years ago

It wouldn't fit the topic of #125, "Developer instructions"

You are able to edit the topic of PR by pressing the edit button on the righthand of the screen.

lmoureaux commented 6 years ago

This would be against:

The feature should be compartmentalized as much as possible

(from page 6 of the manifesto)

bdorney commented 6 years ago

Update to match central and reopen PR.

lmoureaux commented 6 years ago

I already rebased my branch on top of #125, but can't reopen a closed PR.

bdorney commented 6 years ago

I already rebased my branch on top of #125, but can't reopen a closed PR.

:shrug: submit a new one