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

Update docs #52

Merged
merged 12 commits into from
Jan 30, 2024
Merged

Update docs #52

merged 12 commits into from
Jan 30, 2024

Conversation

felixhekhorn
Copy link
Contributor

Closes #4

@comane and @t7phy can you please help? @andreab1997 maybe as well? just improve with your recent experience ...

e.g. we could consider adding the ssl discussion/snippet from #50 somewhere

@felixhekhorn felixhekhorn added the documentation Improvements or additions to documentation label Nov 22, 2023
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (dcd91cb) 29.18% compared to head (b6640bd) 29.18%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #52   +/-   ##
=======================================
  Coverage   29.18%   29.18%           
=======================================
  Files          24       24           
  Lines        1261     1261           
=======================================
  Hits          368      368           
  Misses        893      893           
Flag Coverage Δ
unittests 29.18% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@comane
Copy link
Member

comane commented Nov 22, 2023

Hi @felixhekhorn , I have added a couple of lines regarding openssl as per #50

@felixhekhorn felixhekhorn marked this pull request as draft November 24, 2023 14:48
@alecandido
Copy link
Member

Hi @felixhekhorn , I have added a couple of lines regarding openssl as per #50

Thanks @comane!

At the moment, the best thing to do is definitely document, since we're trying to converge on a stable version. However, a proper solution to it will be #7 (most likely, since the only one using OpenSSL should be Git)

@felixhekhorn
Copy link
Contributor Author

@comane the moment you have updated the mg5 part, I'm ready for review;

(Don't hesitate to reshuffle that part completely - as you can see I already did it so far, you can do it again)

@felixhekhorn
Copy link
Contributor Author

since the only one using OpenSSL should be Git

Actually it is cargo requiring ssl - so the solution is to use the pre-built binaries

@felixhekhorn felixhekhorn marked this pull request as ready for review January 19, 2024 12:50
@felixhekhorn
Copy link
Contributor Author

felixhekhorn commented Jan 19, 2024

let's merge something

@comane if you have time please improve whatever you can (also in a later PR)

@cschwan please have a look (even a quick one) and then we simply merge

@t7phy
Copy link
Member

t7phy commented Jan 19, 2024

Closes #4

@comane and @t7phy can you please help? @andreab1997 maybe as well? just improve with your recent experience ...

e.g. we could consider adding the ssl discussion/snippet from #50 somewhere

@felixhekhorn sorry I must have somehow missed this notification and just saw this. I have actually never used pinefarm

@felixhekhorn
Copy link
Contributor Author

@felixhekhorn sorry I must have somehow missed this notification and just saw this. I have actually never used pinefarm

@t7phy but you are running Matrix, right? so please consider seriously contributing to NNPDF/pinecards#161 (which should really be a pinefarm PR)

@t7phy
Copy link
Member

t7phy commented Jan 19, 2024

@t7phy but you are running Matrix, right? so please consider seriously contributing to NNPDF/pinecards#161 (which should really be a pinefarm PR)

that is true, however while using matrix, personally, I have to always do lots of tweaking based on the requirements of specific run, and this is very situation based and not straightforward or trivial and I am unsure of how much of this can be automated. perhaps we can discuss this some day on how to go about this (based on all the bits and pieces that need to be thought about for every run individually)

@comane
Copy link
Member

comane commented Jan 19, 2024

@comane if you have time please improve whatever you can (also in a later PR)

Hi @felixhekhorn, I don't think I can actually contribute much to this PR.
I did not end up using pinefarm (madgralh) in the end. The only things I had noticed I already wrote down.
Also for Matrix as @t7phy I used the Matrix code rather than pinefarm

@alecandido alecandido removed their request for review January 19, 2024 17:41
@felixhekhorn felixhekhorn merged commit 0609767 into main Jan 30, 2024
3 of 4 checks passed
@felixhekhorn felixhekhorn deleted the update-docs branch January 30, 2024 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update README
4 participants