Skip to content
This repository has been archived by the owner on Jul 12, 2022. It is now read-only.

Add rit update workspace core command #922

Merged

Conversation

GuillaumeFalourd
Copy link
Contributor

@GuillaumeFalourd GuillaumeFalourd commented May 12, 2021

Description

  • Add the rit update workspace core command to allow users to update the workspace tree.json file and access new formulas (or formulas' updates) after updating the workspace using git commands.

How to verify it

  • Install Ritchie CLI.
  • Use this branch locally.
  • Overwrite your current binary following this guide.
  • Execute the rit update workspace command

Demo

rit update workspace

rit.update.workspace.mp4

Changelog

  • Add the rit update workspace core command.

Tasks

Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
@GuillaumeFalourd GuillaumeFalourd added ✨ feature Suggest a new feature or enhancement to the Ritchie project 🚧 WIP Work in Progress labels May 12, 2021
@GuillaumeFalourd GuillaumeFalourd self-assigned this May 12, 2021
@GuillaumeFalourd GuillaumeFalourd linked an issue May 12, 2021 that may be closed by this pull request
1 task
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Copy link
Contributor

@henriquemoraeszup henriquemoraeszup left a comment

Choose a reason for hiding this comment

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

Nice, looks like it is on the right place

pkg/formula/workspace/workspace.go Outdated Show resolved Hide resolved
Comment on lines +113 to +114
split := strings.Split(selected, " (")
workspaceName := split[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have a safer way of doing this? Maybe by the index on the list or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The List method returns an element like this: Default (/Users/username/ritchie-formulas-local)
Therefore this is the easiest way to get the workspace's name based on the user choice (as we don't know how he wrote the workspace's name when he added it).

We could eventually refactor the List method, but it would be necessary to change other core commands implementations. I suggest to do it as another improvement feature PR to not mix things.

Do you agree?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have thought of some workarounds that may be ok.

For instance, let us think that we have a workspace named Fernando (Test). With current implementation code will break, because split will give us 3 slices: Fernando, Test) and path.

Hoping that the user does not use any spaces on directories, we can find workspaceName by joining the recently split slice, but ignoring the last element, that should be the path.

Another approach is that instead of looking for the workspaceName we always look for the workspacePath by getting the last element of split (and cleaning remaining closing brackets).
We check if it is a valid path, if true we join remaining slices in order to get workspaceName.
If it is not a valid path we loop split backwards, joining the elements until we get a valid workspacePath.

What do you think about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have any problem updating the way we list workspaces.
I just think it shouldn't be done in this PR as it will impact 2 others core commands (rit list workspace and rit delete workspace) as well as rit update workspace.

pkg/formula/workspace/workspace.go Outdated Show resolved Hide resolved
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
@codecov-commenter
Copy link

codecov-commenter commented May 14, 2021

Codecov Report

Merging #922 (d7f7638) into master (649431e) will increase coverage by 0.07%.
The diff coverage is 90.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #922      +/-   ##
==========================================
+ Coverage   85.55%   85.62%   +0.07%     
==========================================
  Files         118      119       +1     
  Lines        4305     4375      +70     
==========================================
+ Hits         3683     3746      +63     
- Misses        417      420       +3     
- Partials      205      209       +4     
Impacted Files Coverage Δ
pkg/formula/workspace/workspace.go 81.48% <88.88%> (+2.11%) ⬆️
pkg/cmd/update_workspace.go 90.19% <90.19%> (ø)
pkg/commands/builder.go 90.68% <100.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 649431e...d7f7638. Read the comment docs.

Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
pkg/cmd/update_workspace_test.go Outdated Show resolved Hide resolved
pkg/cmd/update_workspace_test.go Outdated Show resolved Hide resolved
Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
@lucasdittrichzup
Copy link
Contributor

🚀

@GuillaumeFalourd GuillaumeFalourd merged commit b2fc8bf into ZupIT:master May 27, 2021
@GuillaumeFalourd GuillaumeFalourd deleted the feature/update-workspace branch May 27, 2021 18:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
✔️ ready-for-review ready for review ✨ feature Suggest a new feature or enhancement to the Ritchie project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add command 'rit update workspaces'
5 participants