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

The Great Refactoring #26

Closed
bragefuglseth opened this issue Aug 28, 2023 · 5 comments
Closed

The Great Refactoring #26

bragefuglseth opened this issue Aug 28, 2023 · 5 comments
Labels
code This isn't visible in the app, but could still use some work

Comments

@bragefuglseth
Copy link
Owner

The current codebase is really messy in places, both style-wise and structurally. I'll have to do some cleanup to make issues like #23 and #16 more realistically solvable. This would hopefully also attract more contributors in the future, lifting the maintenance burden a little.

@bragefuglseth bragefuglseth added the code This isn't visible in the app, but could still use some work label Aug 28, 2023
@orowith2os
Copy link

Quick suggestions:

  • Move UI files and whatnot to data/resources/ui, rather than src - imo, it's cleaner.
  • Migrate to Blueprint for the UI files
  • Use an array rather than a Vec when you know the size isn't going to change at runtime - see the "Developers" field in the about window for the primary case where this isn't done. I need to submit a fix to the GTK template and (probably) elsewhere for this.
  • The about window builder can be one clean chain of function calls - call .present() on .build(), omit the semicolon, and don't initialize it as a variable. The .transient_for() method can just be this: .transient_for(&self.main_window()). This is what mine looks like, for reference:
    fn show_about_dialog(&self) {
        adw::AboutWindow::builder()
            .modal(true)
            .version(VERSION)
            .transient_for(&self.main_window())
            .application_name(gettext("Toggle"))
            .application_icon(APP_ID)
            .license_type(gtk::License::Gpl30)
            .copyright("© 2023 Dallas Strouse")
            .debug_info(crate::get_debug_info())
            .debug_info_filename("toggle-debuginfo.txt")
            // TODO: set up a website
            .website("https://gitlab.com/orowith2os/toggle")
            .issue_url("https://gitlab.com/orowith2os/toggle/-/issues")
            // TODO: add a link to a public Matrix channel
            //.support_url("")
            .translator_credits(gettext("translator-credits"))
            // To contributors: Feel free to add your name to .developers(),
            // and for those involved with UI, add yourself to .designers()
            .developer_name("Dallas Strouse, et al.")
            .developers(["Dallas Strouse"])
            .designers(["Bart Gravendeel", "Brage Fuglseth"])
            .build()
            .present()
    }

I need to see if one can reuse the release notes from the appropriate appdata file, rather than embedding them in the source code. I'm sure it can be hacked together with Meson.

@bragefuglseth
Copy link
Owner Author

Thanks for the suggestions! Most of that is related to the template, fortunately, but I’ll fix it 😄

@orowith2os
Copy link

Most of that is related to the template, fortunately, but I’ll fix it

I need to see if I can submit changes for both the gtk-rust-template and the GNOME Builder template, to make them similar and a bit nicer - but from what I've heard from @BrainBlasted, for Builder, that's not too solvable.

@orowith2os
Copy link

I need to see if one can reuse the release notes from the appropriate appdata file, rather than embedding them in the source code.

Yep. Unstable, but it's doable. https://gnome.pages.gitlab.gnome.org/libadwaita/doc/main/func.show_about_window_from_appdata.html

@bragefuglseth
Copy link
Owner Author

I think this issue is too vague to actually be actionable. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code This isn't visible in the app, but could still use some work
Projects
None yet
Development

No branches or pull requests

2 participants