Change the default artifact to None#526
Merged
sdglitched merged 1 commit intomainfrom Apr 12, 2026
Merged
Conversation
Reviewer's GuideMake the 'None' artifact the default option in the registry and UI, adjust tests to expect/drive this new default behavior, ensure wipe operations emit the appropriate Qt signals when already on 'None', and configure Qt to run offscreen in tox. Sequence diagram for wiping an artifact and ensuring Qt signal emissionsequenceDiagram
actor User
participant WindRule as WindRule
participant QtCombo as QtComboBox_arti_part_type
User->>WindRule: wipe_artifact(part)
WindRule->>QtCombo: getattr(self, arti_part_type)
WindRule->>QtCombo: currentText()
alt currentText is None
WindRule->>QtCombo: currentTextChanged.emit(None)
else currentText is not None
WindRule->>QtCombo: setCurrentText(None)
end
Sequence diagram for wiping a team of artifact slotssequenceDiagram
actor User
participant WindRule as WindRule
participant QtCombo_fwol as QtComboBox_arti_fwol_type
participant QtCombo_pmod as QtComboBox_arti_pmod_type
participant QtCombo_sdoe as QtComboBox_arti_sdoe_type
participant QtCombo_gboe as QtComboBox_arti_gboe_type
participant QtCombo_ccol as QtComboBox_arti_ccol_type
User->>WindRule: wipe_team()
loop for each slot in [fwol, pmod, sdoe, gboe, ccol]
WindRule->>QtCombo_fwol: currentText() / currentTextChanged.emit or setCurrentText(None)
WindRule->>QtCombo_pmod: currentText() / currentTextChanged.emit or setCurrentText(None)
WindRule->>QtCombo_sdoe: currentText() / currentTextChanged.emit or setCurrentText(None)
WindRule->>QtCombo_gboe: currentText() / currentTextChanged.emit or setCurrentText(None)
WindRule->>QtCombo_ccol: currentText() / currentTextChanged.emit or setCurrentText(None)
end
Flow diagram for artifact registry defaulting to Noneflowchart TB
subgraph ArtifactModule
A[Import none artifact module]
B[Import all other artifact modules]
C[Define ArtifactTeam for none]
D[Populate artilist dict]
end
A --> C
B --> D
C --> D
subgraph ArtiListDict[__artilist__]
N[Key none.team.__teamname__ mapped first]
O[Other artifact teams mapped after]
end
D --> N
D --> O
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- If other code depends on the first entry of
__artilist__as the default artifact, consider making that dependency explicit (e.g., aDEFAULT_ARTIFACTconstant or a dedicated accessor) instead of relying on dictionary insertion order for clarity and robustness.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- If other code depends on the first entry of `__artilist__` as the default artifact, consider making that dependency explicit (e.g., a `DEFAULT_ARTIFACT` constant or a dedicated accessor) instead of relying on dictionary insertion order for clarity and robustness.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
gridhead
force-pushed
the
naan
branch
2 times, most recently
from
f458153 to
bfdd432
Compare
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
wipe_artifactandwipe_team, consider simplifying thecurrentText == "None"special-case and directcurrentTextChanged.emit("None")logic, or at least add a brief comment explaining why bypassingsetCurrentText("None")is required here, since Qt normally emits the signal onsetCurrentTextand this branch may be surprising to future maintainers. - Several of the tests now use standalone triple-quoted strings (e.g. in
test_arti_save_fail,test_team_save_fail,test_otpt_fail) purely as comments; replacing these with#comments would avoid creating no-op string literals and keep intent clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `wipe_artifact` and `wipe_team`, consider simplifying the `currentText == "None"` special-case and direct `currentTextChanged.emit("None")` logic, or at least add a brief comment explaining why bypassing `setCurrentText("None")` is required here, since Qt normally emits the signal on `setCurrentText` and this branch may be surprising to future maintainers.
- Several of the tests now use standalone triple-quoted strings (e.g. in `test_arti_save_fail`, `test_team_save_fail`, `test_otpt_fail`) purely as comments; replacing these with `#` comments would avoid creating no-op string literals and keep intent clearer.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Akashdeep Dhar <akashdeep.dhar@gmail.com>
sdglitched
approved these changes
Apr 12, 2026
Collaborator
sdglitched
left a comment
sdglitched
left a comment
There was a problem hiding this comment.
LGTM 🚀
Nice catching the long-lived problem.
gridhead
pushed a commit
that referenced
this pull request
Apr 12, 2026
Change the default artifact to `None`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change the default artifact to
NoneFixes #499
Summary by Sourcery
Set the placeholder "None" artifact as the default selection and align UI behavior, tests, and CI configuration with that behavior.
Bug Fixes:
Enhancements:
Build:
Tests: