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.

Sign up for GitHub

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

Jump to bottom

Fix/2866/ccsh modify behavior 1 and 3 #3446

Merged
merged 11 commits into from
Dec 18, 2023
Merged

Fix/2866/ccsh modify behavior 1 and 3 #3446

merged 11 commits into from
Dec 18, 2023

Conversation

fritschldwg
Copy link
Contributor

@fritschldwg fritschldwg commented Dec 13, 2023 •
edited
Loading

Fix/2866/ccsh modify behavior 1 and 3

Issue: #2866

Description

Descriptive pull request text, answering:

  • The path specified to remove a node can now begin with a slash across shells
  • Nodes with the same name in different subfolders get handled correctly in the removal process

Definition of Done

A PR is only ready for merge once all the following acceptance criteria are fulfilled:

  • Changes have been manually tested
  • All TODOs related to this PR have been closed
  • There are automated tests for newly written code and bug fixes
  • All bugs discovered while working on this PR have been submitted as issues (if not already an open issue)
  • Documentation (GH-pages, analysis/visualization READMEs, parser READMEs, --help, etc.) has been updated (almost always necessary except for bug fixes)
  • CHANGELOG.md has been updated

@fritschldwg fritschldwg mentioned this pull request Dec 13, 2023
Closed
3 tasks
@fritschldwg fritschldwg force-pushed the fix/2866/ccsh-modify-behavior-1 branch from d211100 to 2995f05 Compare December 13, 2023 12:30
@fritschldwg
Copy link
Contributor Author

  • This PR also fixes the bug related to ubuntu
  • Regarding an example in the issue: Specifying a path for removal as follows: path\to\remove won't work (just like before)

@fritschldwg fritschldwg force-pushed the fix/2866/ccsh-modify-behavior-1 branch from 0ebc0ee to d7ee31f Compare December 14, 2023 10:32
Nereboss
Nereboss reviewed Dec 15, 2023
View reviewed changes
Copy link
Collaborator

@Nereboss Nereboss left a comment

Choose a reason for hiding this comment

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

only small nitpicks

CHANGELOG.md Outdated Show resolved Hide resolved
...eModifier/src/main/kotlin/de/maibornwolff/codecharta/filter/structuremodifier/NodeRemover.kt Outdated Show resolved Hide resolved
...ifier/src/test/kotlin/de/maibornwolff/codecharta/filter/structuremodifier/NodeRemoverTest.kt Outdated Show resolved Hide resolved
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Dec 18, 2023

Quality Gate Passed Quality Gate passed for 'CodeCharta Visualization'

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Dec 18, 2023

Quality Gate Passed Quality Gate passed for 'CodeCharta Analysis'

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Nereboss
Nereboss approved these changes Dec 18, 2023
View reviewed changes
Copy link
Collaborator

@Nereboss Nereboss left a comment

Choose a reason for hiding this comment

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

LGTM!🚀

@fritschldwg fritschldwg merged commit dced705 into main Dec 18, 2023
7 checks passed
@fritschldwg fritschldwg deleted the fix/2866/ccsh-modify-behavior-1 branch December 18, 2023 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ce-bo ce-bo ce-bo left review comments

@Nereboss Nereboss Nereboss approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@fritschldwg @ce-bo @Nereboss