Skip to content

cxx-qt-lib: standardize constructor method naming conventions #1220

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jnbooth
Copy link
Contributor

@jnbooth jnbooth commented Mar 8, 2025

Per #1203, constructor methods are now named directly after their parameters, and suffixed by _opt if they return Option<Self>.

This PR also adds the AnyDateFormat type (an enum of either a DateFormat or a QString) in order to deduplicate methods for converting between date/time types and QStrings:

// Old
QDate::from_string(mystring, &QString::from("myformat"));
QDate::from_string_enum(mystring, DateFormat::ISODate);
date.format(&QString::from("myformat"));
date.format_enum(DateFormat::ISODate);

// New
QDate::from_qstring_opt(mystring, &QString::from("myformat"));
QDate::from_qstring_opt(mystring, DateFormat::ISODate);
date.to_qstring(&QString::from("myformat"));
date.to_qstring(DateFormat::ISODate);

Alternatively, we could get rid of AnyDateFormat and use date.to_qstring_qstring, QDateTime::from_qstring_dateformat_opt, etc. I think the AnyDateFormat approach is slightly cleaner, but I'll get rid of it if you prefer.

These are all breaking changes. Alternatively, the old functions could be deprecated rather than removed. I didn't see any #[deprecated] tags in the codebase, so I defaulted to removing them.

Full list of changes (copied from the changelog):

  • QTime::from_string and QTime::from_string_enum now return Option<QTime> rather than QTime, in keeping with the identical functions for QDate and QDateTime.
  • Combined and renamed the following functions:
    • QDate::from_string and QDate::from_string_enumQDate::from_qstring_opt
    • QDate::format and QDate::format_enumQDate::to_qstring
    • QDateTime::from_date_and_time_time_zoneQDateTime::from_qdate_qtime_qtimezone
    • QDateTime::from_date_and_time_time_specQDateTime::from_qdate_qtime_timespec
    • QDateTime::from_string and QDateTime::from_string_enumQDateTime::from_qstring_opt
    • QDateTime::format and QDateTime::format_enumQDateTime::to_qstring
    • QImage::from_dataQImage::from_data_opt
    • QTime::from_string and QTime::from_string_enumQTime::from_qstring_opt
    • QTime::format and QTime::format_enumQTime::to_qstring

Copy link

codecov bot commented Mar 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (1e80af7) to head (e903c02).

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1220   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           73        73           
  Lines        12593     12593           
=========================================
  Hits         12593     12593           

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant