Skip to content

Conversation

@ebariaux
Copy link
Contributor

No description provided.

@ebariaux ebariaux linked an issue Apr 10, 2025 that may be closed by this pull request
@ebariaux ebariaux marked this pull request as draft April 10, 2025 10:26
# Conflicts:
#	ORLib/Utils/String+Utils.swift
Copy link

@Miggets7 Miggets7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It look all good and very solid. The only thing I'm not sure about if it should be an extension method on the String class or a class method which takes in a String or maybe even an init method? This because it's quite specific to this app and not as generic because of the appending of .openremote.app. Just sharing my thoughts.

@ebariaux
Copy link
Contributor Author

Thanks.
I had it as a method taking a String parameter first (on ConfigManager IIRC), then thought above moving it to a specific utility class and eventually went for the extension to make the calling site look simpler.
Looking at it in more details, ConfigManager should probably be moved from ORLib to GenericApp and then that method would be in an extension living in the app and not the library.

I'll keep it like that for now and create a separate issue to move ConfigManager.

@ebariaux ebariaux merged commit a5a183a into main Apr 11, 2025
@ebariaux ebariaux deleted the feature/DomainParsingFix branch April 11, 2025 12:36
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.

Entering domain without scheme prefix crashes the app

3 participants