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

New PR label for HLT integration #2192

Closed
antoniovilela opened this issue Mar 7, 2024 · 14 comments · Fixed by #2200
Closed

New PR label for HLT integration #2192

antoniovilela opened this issue Mar 7, 2024 · 14 comments · Fixed by #2200

Comments

@antoniovilela
Copy link

@makortel @iarspider @aandvalenzuela

I was wondering if it would be possible to add a label, e.g. "hlt-int", to be added with "type hlt-int" or analogous to a PR.

This would identify work needed for HLT development&integration prior to a data taking period. This is characterized by development performed on top of the production release that needs a faster integration time than other updates to the production branch.

Thanks.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 7, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 7, 2024

A new Issue was created by @antoniovilela.

@smuzaffar, @rappoccio, @makortel, @Dr15Jones, @sextonkennedy, @antoniovilela can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@makortel
Copy link
Contributor

makortel commented Mar 7, 2024

I have a few questions just to understand the situation better:

Is the current urgent label not good enough? (e.g. "too much" other stuff gets labeled with urgent)

Is the "HLT development&integration prior to a data taking period" the only use case that would need to be marked differently from urgent for the "faster integration time"? What about PRs like fixes for crashes in HLT or Tier0? Are existing labels (e.g. combination of urgent and bug-fix) sufficient for those?

(I don't object, but maybe a spelling out hlt-integration would be better. We could also add a type hlt-int shorthand command.)

@antoniovilela
Copy link
Author

I have a few questions just to understand the situation better:

Is the current urgent label not good enough? (e.g. "too much" other stuff gets labeled with urgent)

Is the "HLT development&integration prior to a data taking period" the only use case that would need to be marked differently from urgent for the "faster integration time"? What about PRs like fixes for crashes in HLT or Tier0? Are existing labels (e.g. combination of urgent and bug-fix) sufficient for those?

(I don't object, but maybe a spelling out hlt-integration would be better. We could also add a type hlt-int shorthand command.)

It seems "urgent" is subjective for this use case. There have been cases where they were not marked urgent but they expected a short turnaround in any case. Type "hlt-integration" would mark this somewhat exceptional HLT development cycle.

For fixing crashes during data taking, the PRs should be marked as urgent, and ORM should be contacted. We would then also bypass the requirement of ORP approval (as drafted in cms-sw/cmssw#44283).

"hlt-integration" with the "hlt-int" shorthand sounds great.

Thanks again.

@cms-sw/hlt-l2

@mmusich
Copy link
Contributor

mmusich commented Mar 7, 2024

There have been cases where they were not marked urgent but they expected a short turnaround in any case

This is indeed why we have been reluctant to mark these development PRs (essentially menu dumps) as "urgent". They are at a different (lower) level of urgency with respect to a fix for a HLT or Tier0 crash, but still we are working on a schedule to deliver the menu in time to allow deployment and testing so they should not lag too far behind.
Additionally to this, there is sometimes urgency in integrating reconstruction developments that directly impinge on the HLT schedule (see alpaka or fillDescriptions fixes that affect confDB parsing).
Let me tag also @missirol (apologies I don't know Silvia's github user).

@missirol
Copy link
Contributor

missirol commented Mar 7, 2024

@goys

@makortel
Copy link
Contributor

makortel commented Mar 8, 2024

So if I understood correctly, the hlt-integration would be less urgent than urgent (*), but urgent-enough to require special attention from ORP?

(*) depending of course how urgent is used in practice

@antoniovilela
Copy link
Author

So if I understood correctly, the hlt-integration would be less urgent than urgent (*), but urgent-enough to require special attention from ORP?

(*) depending of course how urgent is used in practice

I would not specify the level of urgency here.

This is simply to help categorize this class of PRs. In practice, it will be useful in order to focus on these PRs, especially during certain periods.

These PRs can eventually also be marked as urgent, on top of this hlt-integration category.

This category will be included in the guidelines, which is however broader than what is being discussed in this particular issue.

Thanks.

@cms-sw/orp-l2

@makortel
Copy link
Contributor

makortel commented Mar 8, 2024

Ok, let's add the hlt-integration type label, with type hlt-int shorthand. I leave it to @smuzaffar @iarspider @aandvalenzuela to figure out who will do the changes.

@mmusich
Copy link
Contributor

mmusich commented Mar 11, 2024

tangentially related to this thread I took the liberty of opening cms-sw/cms-sw.github.io#118, this might help developers to produce code that can be directly used in HLT configurations.

@antoniovilela
Copy link
Author

Ok, let's add the hlt-integration type label, with type hlt-int shorthand. I leave it to @smuzaffar @iarspider @aandvalenzuela to figure out who will do the changes.

@smuzaffar
Hi Shahzad,
Does this look ok to you?
Thanks.

@antoniovilela
Copy link
Author

@smuzaffar @aandvalenzuela @iarspider
All ok to add this label?
Thanks.

@smuzaffar
Copy link
Contributor

@antoniovilela , yes all ok. I will open a PR now

@antoniovilela
Copy link
Author

@antoniovilela , yes all ok. I will open a PR now

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants