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

validator: Reduce severity of release-time-missing on snapshot releases #650

Conversation

jayaddison
Copy link
Contributor

Resolves #570.

@jayaddison jayaddison force-pushed the issue-570/skip-release-time-missing-for-snapshots branch 2 times, most recently from 2f54866 to 07d6bda Compare August 18, 2024 21:57
@ximion
Copy link
Owner

ximion commented Aug 20, 2024

This is not quite what I meant in the bug report...
If there is a date defined, then we should absolutely validate it. But for snapshot releases, having no date shouldn't be an error.

Since there may be a release tomorrow I may just implement that change quickly (but not if the release is deferred to Thursday).

@jayaddison

This comment was marked as outdated.

@jayaddison

This comment was marked as outdated.

@jayaddison

This comment was marked as outdated.

@jayaddison jayaddison changed the title validator: Omit date validation for snapshot releases validator: Reduce severity of release-time-missing on snapshot releases Aug 28, 2024
@jayaddison

This comment was marked as resolved.

@jayaddison

This comment was marked as resolved.

@jayaddison

This comment was marked as resolved.

@jayaddison jayaddison force-pushed the issue-570/skip-release-time-missing-for-snapshots branch from 089f4d8 to 4bda793 Compare August 28, 2024 13:21
@jayaddison

This comment was marked as resolved.

@jayaddison

This comment was marked as resolved.

@jayaddison

This comment was marked as resolved.

@jayaddison
Copy link
Contributor Author

jayaddison commented Aug 28, 2024

@ximion I think that this changeset now adheres more-closely to what you suggested in #570 and here: specifically that although release-time-missing should not be reported as an error for snapshot releases, we should still run the relevant validation and inform users of possible problems.

@jayaddison
Copy link
Contributor Author

@ximion my communication on this thread was somewhat verbose, so I'll try to be concise: I'd be glad for any code review on the approach currently coded here (approximately: refactor the issue-emission code to allow a severity to be passed by-argument, and use that functionality to emit a reduced-severity (warning instead of error) notice for release-time-missing in the specific case of snapshot releases).

@jayaddison
Copy link
Contributor Author

Sorry to nag: is this likely to be reviewable in the nearish future? I'm very much not a competent C++ developer, so if the changes initially seem rough/unacceptable, I'd probably prefer to close it and await a more robust implementation.

@jayaddison
Copy link
Contributor Author

Closing to reduce my mental overhead of open tasks -- not discouraged about reaching a solution for this, just trying to refocus. My eventual hope with resolution of #570 is that I can re-enable the release-time validation for gnome-maps (disabled (hopefully temporarily!) as the warnings conflicted with a build-reproducibility fix).

@jayaddison jayaddison closed this Nov 5, 2024
@jayaddison jayaddison deleted the issue-570/skip-release-time-missing-for-snapshots branch November 5, 2024 02:13
ximion added a commit that referenced this pull request Dec 8, 2024
Resolves: #570, #650

Thanks to James Addison for the initial work!
@ximion
Copy link
Owner

ximion commented Dec 8, 2024

I am very sorry for getting back to this so late and not supervising this PR properly. I had to take some absence from FOSS development for personal reasons.

But the actual change was trivial, sorry that you put so much more work into it than necessary! It will ship with the next release :-)

@jayaddison
Copy link
Contributor Author

No problem at all - real life happens, and should take priority.

I'm very grateful for 13a94d5 - yep, I got a bit frantic attempting an implementation, but it was fun to try some C again, and I learned some things :) Thanks again!

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.

How can app handle unrelased release date in metada
2 participants