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

feature: support autolayout in seata-statemachine-designer #6415

Merged
merged 23 commits into from
Mar 16, 2024

Conversation

Code-breaker1998
Copy link
Contributor

…gner

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

      These PR resolve the issue of autolayout in apache seata statemachine diagram

Ⅱ. Does this pull request fix one issue?

    fixes #6136 

Ⅲ. Why don't you add test cases (unit test/integration test)?

    following are the test cases under which I perform testing

sega1.json
sega2.json
sega3.json
sega4.json
StateMachine-6grdofu.json

Ⅳ. Describe how to verify it

   Click on import option , select the file which does not contain property like style,edge and waypoints . check whether they are able to import in statemachine diagram

Ⅴ. Special notes for reviews

@Code-breaker1998
Copy link
Contributor Author

@ptyin ,
Can you go through this PR?

Copy link
Contributor

@liuqiufeng liuqiufeng left a comment

Choose a reason for hiding this comment

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

plz ignore .factorypath file

Copy link
Contributor

@liuqiufeng liuqiufeng left a comment

Choose a reason for hiding this comment

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

plz do not submit the package-lock.json file without a package update

@Code-breaker1998
Copy link
Contributor Author

should i submit new PR or Will be fine with these...

@liuqiufeng
Copy link
Contributor

plz ignore .

should i submit new PR or Will be fine with these...

可以直接在关联分支上改,提交后pr会同步生效
You can change it directly on the associated branch, and pr will be synchronized after the commit.

@codecov-commenter
Copy link

codecov-commenter commented Mar 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 50.41%. Comparing base (f93a721) to head (460fbc4).
Report is 1 commits behind head on 2.x.

❗ Current head 460fbc4 differs from pull request most recent head 1bc9fbb. Consider uploading reports for the commit 1bc9fbb to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #6415      +/-   ##
============================================
- Coverage     50.43%   50.41%   -0.02%     
  Complexity     5253     5253              
============================================
  Files           941      941              
  Lines         33279    33279              
  Branches       4035     4035              
============================================
- Hits          16783    16778       -5     
- Misses        14871    14876       +5     
  Partials       1625     1625              

see 5 files with indirect coverage changes

@ptyin ptyin changed the title issue: Resolve issue related to autolayout in seata-statemachine-desi… feature: support autolayout in seata-statemachine-designer Mar 12, 2024
Copy link
Member

@ptyin ptyin left a comment

Choose a reason for hiding this comment

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

Fantastic job! I tested some JSON locally, and it works. There are some minor problems. PTAL.

However, some of auto-generated layout may look bizarre. I think we can sort it out in the future work. If you are interested in make the layout prettier, you can work on it and submit another PR (only if you feel that idea interested and worth trying).
image

@Code-breaker1998
Copy link
Contributor Author

@ptyin

We can surely improve layout more , but for only can you accept these feature , if in future some one found difficulty in reading layout I will try once again .

I have resolved the changes can you check it once again.

Copy link
Member

@ptyin ptyin left a comment

Choose a reason for hiding this comment

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

Basically there are too many eslint and code style problems. But no worry, I am working on fixing these problems. Maybe you can pay more attention next time.

@ptyin
Copy link
Member

ptyin commented Mar 13, 2024

I will go over this PR again. There are some other minor necessary changes.

@Code-breaker1998
Copy link
Contributor Author

sure

@ptyin
Copy link
Member

ptyin commented Mar 15, 2024

@Code-breaker1998 Hi, are you working on it?

@Code-breaker1998
Copy link
Contributor Author

Yes I am working on it . Give me 1 hour I will submit it

@ptyin
Copy link
Member

ptyin commented Mar 15, 2024

Yes I am working on it . Give me 1 hour I will submit it

No hurry, take your time.

@Code-breaker1998
Copy link
Contributor Author

@ptyin , done with the changes , please go through it

Copy link
Member

@ptyin ptyin left a comment

Choose a reason for hiding this comment

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

LGTM

@ptyin ptyin merged commit a0b2d9b into apache:2.x Mar 16, 2024
4 of 6 checks passed
@ptyin
Copy link
Member

ptyin commented Mar 16, 2024

@Code-breaker1998 Congratz! Your first PR has been successfully merged. Thanks for your work.

@Code-breaker1998
Copy link
Contributor Author

Thanks, for your cooperation
But I have one question why you were telling me to use 'is' function , as without that also thing were getting done.

@ptyin
Copy link
Member

ptyin commented Mar 16, 2024 via email

@funky-eyes funky-eyes added this to the 2.1.0 milestone Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants