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

Report work on objectives instead of workitems #218

Merged
merged 16 commits into from
May 23, 2024

Conversation

gpetiot
Copy link
Member

@gpetiot gpetiot commented Apr 16, 2024

fix #167
also fixes #159

I've been trying a few things, the issue in making the linter more strict right now, is that older reports won't pass the linting unless we maintain a double-check (depending on the submitted date for example, e.g. like versioning the checks), which is the same problem as linting files that contain closed workitems, these files should pass the linting, but we should print a warning (see #170).

So I've opted for printing warnings when reporting on a workitem instead of an objective, and telling the user which objective to use if there is one.

The examples are in test/cram/lint/db.t

What do you think?

@gpetiot gpetiot marked this pull request as draft April 16, 2024 17:17
@samoht
Copy link
Contributor

samoht commented Apr 16, 2024

I think it might be ok to "upgrade" existing weeklies - ie the parser could accept WIs but the printer will print the corresponding objective. Ie it's ok to update the data (with some automatic translation) as well as the tool.

@rikusilvola
Copy link
Member

@samoht that might create issues with long-standing Work Items that get moved from objective to objective, e.g. Comp65.

@gpetiot gpetiot force-pushed the report-on-objectives branch 2 times, most recently from 3fd9066 to 7adf959 Compare April 29, 2024 14:09
@gpetiot
Copy link
Member Author

gpetiot commented Apr 29, 2024

@samoht that might create issues with long-standing Work Items that get moved from objective to objective, e.g. Comp65.

This sounds like an issue that would be very specific to Comp65, I know the scale of the project makes it hard to compare to other projects where the maintenance objective is the same year on year (like it's the case for ocamlformat and odoc). But still, for this to work we need to have 1 objective associated to N workitems, not the opposite.

@rikusilvola is it feasible to make sure the compiler workitems and objectives comply with this rule? A soft upgrade like what samoht proposed sounds like the best idea to me.

@gpetiot gpetiot marked this pull request as ready for review April 29, 2024 14:58
@gpetiot gpetiot changed the title WIP: Report work on objectives instead of workitems Report work on objectives instead of workitems Apr 29, 2024
@gpetiot
Copy link
Member Author

gpetiot commented Apr 29, 2024

Once this is merged we should probably release okra.1.0.0

@rikusilvola
Copy link
Member

rikusilvola commented Apr 29, 2024

@rikusilvola is it feasible to make sure the compiler workitems and objectives comply with this rule? A soft upgrade like what samoht proposed sounds like the best idea to me.

No WI should be on two objectives anyway, just that they tend to move from objective to objective.

I guess it's fine since it won't affect older weeklies. By 2025, we ought to all be reporting on objectives instead, and the Comp65 and other rolling WIs will be closed.

@gpetiot
Copy link
Member Author

gpetiot commented May 17, 2024

@rikusilvola @jmid the PR is ready for review and experimentation if you have time to try out this branch

@gpetiot
Copy link
Member Author

gpetiot commented May 20, 2024

I've added tests for cat and team aggregate and everything is working as expected. Does anyone have objections before I merge it? @samoht @kayceesrk @rikusilvola @jmid @balat @wafaesara

@gpetiot gpetiot merged commit 029c221 into tarides:main May 23, 2024
3 checks passed
@gpetiot gpetiot deleted the report-on-objectives branch May 23, 2024 09:12
gpetiot added a commit to gpetiot/tarides-opam-repository that referenced this pull request May 29, 2024
CHANGES:

### Changed

- Lint: activity is now checked against objectives instead of workitems (tarides/okra#218, @gpetiot)
- Cat/Aggregate: generated reports use objectives instead of workitems (tarides/okra#218, @gpetiot)
- Use standardized categories for reporting non-engineering work time: Community, Hack, Learning, Leave, Management, Meet and Onboard (tarides/okra#230, @gpetiot)
- Lint: check the quarter of workitems instead of their status (tarides/okra#228, @gpetiot)
- Normalize error messages (tarides/okra#237, tarides/okra#239, @gpetiot)

### Added

- Allow engineers to specify the number of working days (tarides/okra#232, @patricoferris)
- Check that [--engineer] and [--team] are not both true (tarides/okra#233, @gpetiot)
- Gen: add example entries for standardized categories (tarides/okra#236, @gpetiot)
gpetiot added a commit to tarides/opam-repository that referenced this pull request May 29, 2024
CHANGES:

### Changed

- Lint: activity is now checked against objectives instead of workitems (tarides/okra#218, @gpetiot)
- Cat/Aggregate: generated reports use objectives instead of workitems (tarides/okra#218, @gpetiot)
- Use standardized categories for reporting non-engineering work time: Community, Hack, Learning, Leave, Management, Meet and Onboard (tarides/okra#230, @gpetiot)
- Lint: check the quarter of workitems instead of their status (tarides/okra#228, @gpetiot)
- Normalize error messages (tarides/okra#237, tarides/okra#239, @gpetiot)

### Added

- Allow engineers to specify the number of working days (tarides/okra#232, @patricoferris)
- Check that [--engineer] and [--team] are not both true (tarides/okra#233, @gpetiot)
- Gen: add example entries for standardized categories (tarides/okra#236, @gpetiot)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report on objectives and not WIs Retire old identifiers and point to new identifiers
3 participants