Skip to content

Conversation

friday
Copy link
Member

@friday friday commented Dec 14, 2021

"Item" doesn't add anything to the specificity or grammatical correctness so it can be removed.

This of course has to be backwards compatible, so the current names used by extension (ExtensionResultItem and ExtensionSmallResultItem) have to keep working for extension at least throughout version 6.x.x, and ResultItem also, for those extensions that use type annotations (it's just one developer doing this, but it's a valid use case at least until #933 is implemented).

  1. Renamed all "ResultItem" and "result_item" references to just "Result" and "result"
  2. Moved the locations while I was at it, to make the imports easier. From from ulauncher.api.shared.item.Result import Result to from ulauncher.api import Result. If you also need more than one Result variant you can now for example import like this: from ulauncher.api import Result, SmallResult instead of two separate lines, and the plan is to move more of these classes into just ulauncher.api rather than a deeper hierchy.
  3. In order to accomplish 2. without conflicts I also had to rename the modules, so instead of naming them Result.py, SmallResult.py etc they're now small_result.py. This is the the pep8 standard anyway "Modules should have short, all-lowercase names. Underscores can be used in the module name if it improves readability. Python packages should also have short, all-lowercase names, although the use of underscores is discouraged.". With this change I used init.py to define the imports with a shorter common path. For memory consumption this pattern could be negative, but not for these four imports.
  4. Added modules in place of the old ResultItem.py etc, importing the new classes and making them accessible to extensions using the old import statements. SmallResult(Item) was intentionally left out as only the other three have valid use cases for extensions. I even checked using global GitHub search to see if any extensions use SmallResult(Item), and none of them do. There's a few small number of extensions that imports it and never uses it, but they will be able to fix that easy.

Checklist

  • Verify that the test command ./ul test is passing (the CI server will check this if you don't)
  • Update the documentation according to your changes (when applicable)
  • Write unit tests for your changes (when applicable)

@friday friday merged commit e4a9912 into v6 Dec 14, 2021
@friday friday deleted the v6-932 branch December 14, 2021 21:10
@friday friday added this to the 6.0.0 milestone Dec 14, 2021
@friday friday mentioned this pull request Mar 29, 2022
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