Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Repository settings UI #1078

Merged
merged 5 commits into from
Jan 7, 2025
Merged

Add Repository settings UI #1078

merged 5 commits into from
Jan 7, 2025

Conversation

minwoox
Copy link
Contributor

@minwoox minwoox commented Dec 27, 2024

Motivation:
To provide users with a dedicated interface for managing repository settings.

Modifications:

  • Added RepositorySettingsPage component for centralized repository settings management.
  • Refactored UI code for improved maintainability and reusability:
    • Consolidated ProjectRole definitions into a single location
    • Merged AppMemberList and AppTokenList into a unified AppEntityList component to reduce code duplication
    • Merged DeleteMember and DeleteToken functionalities into a single DeleteAppEntity component to reduce code duplication
  • Implemented repository-level credential APIs to support new settings features

Result:

  • New RepositorySettingsPage accessible to users for managing repository-specific settings.

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

👍👍

return credentials(loginUser, future);
}

private static CompletableFuture<List<Credential>> credentials(User loginUser,
Copy link
Contributor

Choose a reason for hiding this comment

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

The method name seems vague. How about maybeMaskSecret()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good name. 👍

* Returns a mirroring credential of the specified {@code id}.
* Returns a list of credentials of the specified repository.
*/
CompletableFuture<List<Credential>> credentials(String repoName);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be good to clarify the function names to improve readability and prevent mistakes. The suggested method names are longer though.

  • repoCredentials()
  • projectCredentials()

@@ -58,7 +58,7 @@

public final class DefaultMetaRepository extends RepositoryWrapper implements MetaRepository {

public static final String PATH_CREDENTIALS = "/credentials/";
private static final String PATH_CREDENTIALS = "/credentials/";

public static final String PATH_MIRRORS = "/mirrors/";
Copy link
Contributor

Choose a reason for hiding this comment

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

Make this field as private as well?

@minwoox minwoox modified the milestones: 0.73.1, 0.74.0 Jan 7, 2025
@minwoox minwoox merged commit 015db56 into line:main Jan 7, 2025
9 of 10 checks passed
@minwoox minwoox deleted the repo_credential branch January 7, 2025 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants