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

Fixes #1310: mason add command ask for overwrite if brick is found #1366

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

koukibadr
Copy link

Status

Ready for review

Description

This is a part of #1310, "feat: ask to overwrite or not for duplicate brick name".

Changes:

  • check if global mason list or local mason list contains the brick name based on add command parameters
  • prompt to overwrite or not the exisiting installed brick (default is set to true)

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@koukibadr koukibadr requested a review from felangel as a code owner July 1, 2024 16:33
Copy link
Owner

Choose a reason for hiding this comment

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

Can we revert the changes to the pubspec since they aren’t related to the issue?

Copy link
Author

Choose a reason for hiding this comment

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

Okay I'll revert the pubspec changes


final masonYaml = isGlobal ? globalMasonYaml : localMasonYaml;
if (masonYaml.bricks.keys.contains(brick.name)) {
final confirmation = logger.confirm(
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add some unit tests for the new functionality?

final masonYaml = isGlobal ? globalMasonYaml : localMasonYaml;
if (masonYaml.bricks.keys.contains(brick.name)) {
final confirmation = logger.confirm(
'Existing ${brick.name} found, you want to overwrite it?',
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
'Existing ${brick.name} found, you want to overwrite it?',
'Existing ${brick.name} found. Do you want to overwrite it?',

@koukibadr koukibadr force-pushed the fixes-#1310-overwrite-option-while-installing branch from 25688ec to cf032d8 Compare July 2, 2024 18:31
@koukibadr koukibadr requested a review from felangel July 2, 2024 18:32
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.

None yet

2 participants