Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Refactoring RepositoryForm Validators #2395

Open
StanleyGoldman wants to merge 1 commit into essentials-publish
base: essentials-publish
Choose a base branch
Loading
from refactoring-publish-validators

Conversation

@StanleyGoldman
Copy link
Contributor

@StanleyGoldman StanleyGoldman commented Jun 27, 2019

No description provided.

await SafeRepositoryNameWarningValidator.ResetAsync();
RepositoryName = name;
});
// this.WhenAny(x => x.SelectedConnection, x => x.SelectedAccount,
Copy link
Collaborator

@jcansdale jcansdale Jun 28, 2019
edited
Loading

Choose a reason for hiding this comment

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

We should delete these comments!

string SafeRepositoryName { get; }
ReactivePropertyValidator<string> RepositoryNameValidator { get; }
ReactivePropertyValidator<string> SafeRepositoryNameWarningValidator { get; }
ReactivePropertyValidator<(string repositoryName, IConnection connection, IAccount account)> RepositoryNameValidator { get; }
Copy link
Collaborator

@jcansdale jcansdale Jun 28, 2019

Choose a reason for hiding this comment

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

Looking at the code, I can't see where we're using the connection or account for validation. I wonder if once upon a time we were checking for an existing repository with the same name as part of the validation? It looks like this check is now done and the error surfaced when the user attempts to create the repository.

I think we might be able to simplify this to use just the repositoryName. I'm wondering if the buggy code was actually completely obsolete. ;-)

Copy link
Collaborator

@jcansdale jcansdale left a comment

Choose a reason for hiding this comment

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

I'm wondering if we could simply remove the buggy code?

See comment:
#2395 (review)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@jcansdale jcansdale jcansdale requested changes

@RobbyLena RobbyLena RobbyLena approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /