swift-testing
Simplify loading tag colors from the home folder.
#117
Merged
Overview
Commits
1
Changes
View On GitHub

Simplify loading tag colors from the home folder. #117

grynspan merged 1 commit into main from jgrynspan/simplify-tag-color-loading
grynspan
grynspan1 year ago

This PR simplifies the code used to load tags from the home folder. In particular, it adds CodingKeyRepresentable conformance to Tag so that tags can be used directly as keys in a JSON-encoded dictionary without us having to jump through hoops to convert them from strings.

grynspan grynspan requested a review from stmontgomery stmontgomery 1 year ago
grynspan grynspan requested a review from dennisweissmann dennisweissmann 1 year ago
grynspan grynspan requested a review from briancroom briancroom 1 year ago
grynspan grynspan requested a review from SeanROlszewski SeanROlszewski 1 year ago
grynspan grynspan requested a review from suzannaratcliff suzannaratcliff 1 year ago
grynspan grynspan requested a review from ejvaughan ejvaughan 1 year ago
grynspan Simplify loading tag colors from the home folder.
8d43c206
grynspan grynspan force pushed to 8d43c206 1 year ago
grynspan
grynspan1 year ago

@swift-ci please test

briancroom
briancroom approved these changes on 2023-11-14
grynspan grynspan merged 322b12c0 into main 1 year ago
grynspan grynspan deleted the jgrynspan/simplify-tag-color-loading branch 1 year ago
stmontgomery stmontgomery assigned grynspan grynspan 1 year ago
stmontgomery stmontgomery added enhancement
stmontgomery
stmontgomery commented on 2023-11-14
Conversation is marked as resolved
Show resolved
Sources/Testing/Running/EntryPoint.swift
260260#endif
261261
262262 // Load tag colors from user/package preferences on disk.
263 result += tagColorOptions()
263
 if let tagColors = try? loadTagColors() {
stmontgomery1 year ago

If there's an error thrown here should we log at least, while proceeding to run tests? (I see this is not really a behavior change from before this PR, but might be nice, still)

grynspan1 year ago

Maybe. I don't want to leak a dependency on Foundation out of that file, but we'd need to check for CocoaError(.fileNotFound) (or however it's spelt.)

stmontgomery1 year ago

Oh I figured it could just handle any error, not necessarily specific ones. Tag colors aren't mission critical, I wouldn't say, so any error that arises could just be noted but we continue to run tests

grynspan1 year ago

That wouldn't work because it throws an error if the user hasn't created a "tag-colors.json" file, which is obviously not a failure mode. Hence we'd need to special-case file-not-found errors.

stmontgomery1 year ago

Maybe separating out the "is there a colors file" and "parse said file" logic would make that easier? Seems like the
former could be non-throwing

grynspan1 year ago

That would require us to leak the Foundation dependency out in the form of returning an instance of Data? which, again, we want to avoid.

Since, as you said, tag colors aren't mission critical, we're probably overthinking it. :)

Login to write a write a comment.

Login via GitHub

Reviewers
Assignees
Labels
enhancement
Milestone
No milestone
Terms Of Use Privacy Policy Refund Policy Impressum