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

main: Set module names as title #429

Merged
merged 1 commit into from
Jun 10, 2024
Merged

Conversation

bbhtt
Copy link
Contributor

@bbhtt bbhtt commented Jun 4, 2024

Choose the "best" title with most module names while making sure it does
not cross 70 characters. The limit is to prevent too much verbosity in
commit messages and PR titles when a large set of modules are getting
updated.

If it crosses 70 characters, start deducting module names and add the
number of remaining updated modules.

@bbhtt
Copy link
Contributor Author

bbhtt commented Jun 4, 2024

56 is the max characters Github can hold in the title in a single line. The commit message is probably 70 or 80.

I'd like to keep some form of "backoff" parameter to prevent ugly PR titles.

I convert it to set to avoid duplicate module names see eg. flathub/org.kde.kdenlive#395

@wjt
Copy link
Contributor

wjt commented Jun 4, 2024

Something like this, but preserving the order of changes?

def build_commit_message(changes: t.List[str]) -> str:
    assert len(changes) >= 1

    if len(changes) == 1:
        return changes[0]

    module_names = list({i.split(":", 1)[0] for i in changes})
    for i in reversed(range(2, len(module_names) + 1)):
        print(i)
        xs = module_names[:i - 1]
        y = module_names[i - 1]
        zs = module_names[i:]

        if zs:
            tail = f" and {len(zs)} more"
            xs.append(y)
        else:
            tail = f" and {y}"

        subject = "Update " + ", ".join(xs) + tail
        if len(subject) <= 56:
            return subject

    return f"Update {len(changes)} modules"

@bbhtt
Copy link
Contributor Author

bbhtt commented Jun 4, 2024

If you want to preserve the order in the list only there might be some shorter way I'll check soon.

list({i.split(":", 1)[0] for i in changes})

This doesn't preserve order of changes

@bbhtt
Copy link
Contributor Author

bbhtt commented Jun 4, 2024

Pushed something that will preserve order. The rest of the formatting I don't have opinions about. But if you can give me it as a patch I can preserve authorship.

@bbhtt
Copy link
Contributor Author

bbhtt commented Jun 4, 2024

Also how about adding Update: a, b and n more *modules* ? As these aren't sources necessarily a single module can have multiple sources with checker data associated.

@bbhtt
Copy link
Contributor Author

bbhtt commented Jun 9, 2024

But if you can give me it as a patch I can preserve authorship.

@wjt Would you prefer that I finish this?

@wjt
Copy link
Contributor

wjt commented Jun 9, 2024

Sure, I don't mind about authorship!

Choose the "best" title with most module names while making sure it does
not cross 70 characters. The limit is to prevent too much verbosity in
commit messages and PR titles when a large set of modules are getting
updated.

If it crosses 70 characters, start deducting module names and add the
number of remaining updated modules.
@bbhtt
Copy link
Contributor Author

bbhtt commented Jun 10, 2024

Done

@bbhtt
Copy link
Contributor Author

bbhtt commented Jun 10, 2024

I changed the limit to 70 characters to allow more module names in the title.

@wjt wjt merged commit e4b970e into flathub-infra:master Jun 10, 2024
3 checks passed
@bbhtt bbhtt deleted the bbhtt/subj-module branch June 10, 2024 14:02
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.

2 participants