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

feat: add deleteByIdBatch interface and my information #247

Merged
merged 7 commits into from
Oct 25, 2023

Conversation

shbone
Copy link
Contributor

@shbone shbone commented Oct 16, 2023

  • add my personal information to pom.xml
  • add add deleteByIdBatch interface function
  • add relative test module

@codecov-commenter
Copy link

codecov-commenter commented Oct 16, 2023

Codecov Report

Attention: 161 lines in your changes are missing coverage. Please review.

Comparison is base (c67c017) 0.00% compared to head (b2ee3d2) 0.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #247   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files          75      75           
  Lines        2563    2563           
  Branches      276     276           
======================================
  Misses       2563    2563           
Files Coverage Δ
...g/nebula/contrib/ngbatis/proxy/NebulaDaoBasic.java 0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shbone
Copy link
Contributor Author

shbone commented Oct 16, 2023

Excuse me! I have a problem. @wey-gu
I only change several lines of code in NebulaDaoBasic.java but there have been
@@ -1,467 +1,487 @@
I don't know how to fix it.


<delete id="deleteByIdBatch">
@for ( v in ng_args[0] ) {
DELETE VERTEX ${ ng.valueFmt( v )};
Copy link
Collaborator

Choose a reason for hiding this comment

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

The use of passing in multiple ids at the same time may be more appropriate than the use of multiple statement structures.
https://docs.nebula-graph.com.cn/3.6.0/3.ngql-guide/12.vertex-statements/4.delete-vertex/

DELETE VERTEX <vid> [ , <vid> ... ] [WITH EDGE];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I learn that and will change it later!

@shbone shbone requested a review from CorvusYe October 22, 2023 15:22
Copy link
Collaborator

@CorvusYe CorvusYe left a comment

Choose a reason for hiding this comment

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

Thank you for your efforts and waiting.
That's very elegant.
We can merge without any additional changes.
cc: @wey-gu

@CorvusYe
Copy link
Collaborator

CorvusYe commented Oct 23, 2023

@shbone
I need to trouble you again to handle the conflict.
And add some infomations in this sections.

ngbatis/CHANGELOG.md

Lines 29 to 44 in c67c017

# NEXT
## Dependencies upgrade
- nebula-java: 3.5.0 -> 3.6.0
## Bugfix
- fix: [#190](https://github.com/nebula-contrib/ngbatis/issues/190) Insert failed when tag has no attributes
- chore: removing and exclude some packages: log4j related or useless.
- fix: [#194](https://github.com/nebula-contrib/ngbatis/issues/194) we can name the interface by `@Component` and `@Resource`, for example:
- `@Component("namedMapper")`: use `@Resource("namedMapper$Proxy")` to inject. (since v1.0)
- `@Resource("namedComponent")`: use `@Resource("namedComponent")` to inject. (new feature)
- fix: when DAO/Mapper method has `Page` type param with `@Param`, the param name can not be use.
> 如原来项目中分页相关接口,用了不起作用的 `@Param`, 但 xml 还是使用 p0, p1...
> 需要将 `@Param` 移除,或者将 xml 中的参数名改成 注解的参数名,以保证参数名统一

Two PRs should belong to ## Features

Your efforts should be perceived by people.

Thank you vvvvvery much!

@shbone
Copy link
Contributor Author

shbone commented Oct 24, 2023

@shbone I need to trouble you again to handle the conflict. And add some infomations in this sections.

ngbatis/CHANGELOG.md

Lines 29 to 44 in c67c017

# NEXT
## Dependencies upgrade
- nebula-java: 3.5.0 -> 3.6.0
## Bugfix
- fix: [#190](https://github.com/nebula-contrib/ngbatis/issues/190) Insert failed when tag has no attributes
- chore: removing and exclude some packages: log4j related or useless.
- fix: [#194](https://github.com/nebula-contrib/ngbatis/issues/194) we can name the interface by `@Component` and `@Resource`, for example:
- `@Component("namedMapper")`: use `@Resource("namedMapper$Proxy")` to inject. (since v1.0)
- `@Resource("namedComponent")`: use `@Resource("namedComponent")` to inject. (new feature)
- fix: when DAO/Mapper method has `Page` type param with `@Param`, the param name can not be use.
> 如原来项目中分页相关接口,用了不起作用的 `@Param`, 但 xml 还是使用 p0, p1...
> 需要将 `@Param` 移除,或者将 xml 中的参数名改成 注解的参数名,以保证参数名统一

Two PRs should belong to ## Features

Your efforts should be perceived by people.

Thank you vvvvvery much!

Thank you ! I will fix this conflict and edit the [ngbatis/CHANGELOG.md]

@shbone
Copy link
Contributor Author

shbone commented Oct 24, 2023

@shbone I need to trouble you again to handle the conflict. And add some infomations in this sections.

ngbatis/CHANGELOG.md

Lines 29 to 44 in c67c017

# NEXT
## Dependencies upgrade
- nebula-java: 3.5.0 -> 3.6.0
## Bugfix
- fix: [#190](https://github.com/nebula-contrib/ngbatis/issues/190) Insert failed when tag has no attributes
- chore: removing and exclude some packages: log4j related or useless.
- fix: [#194](https://github.com/nebula-contrib/ngbatis/issues/194) we can name the interface by `@Component` and `@Resource`, for example:
- `@Component("namedMapper")`: use `@Resource("namedMapper$Proxy")` to inject. (since v1.0)
- `@Resource("namedComponent")`: use `@Resource("namedComponent")` to inject. (new feature)
- fix: when DAO/Mapper method has `Page` type param with `@Param`, the param name can not be use.
> 如原来项目中分页相关接口,用了不起作用的 `@Param`, 但 xml 还是使用 p0, p1...
> 需要将 `@Param` 移除,或者将 xml 中的参数名改成 注解的参数名,以保证参数名统一

Two PRs should belong to ## Features

Your efforts should be perceived by people.

Thank you vvvvvery much!

And I am sorry that I don't know why I have this conflict between src/main/java/org/nebula/contrib/ngbatis/proxy/NebulaDaoBasic.java and src/main/resources/NebulaDaoBasic.xml. It is hard for me, whether I should delete the Param and change it to List<> ? Thank you!!

@CorvusYe
Copy link
Collaborator

This might have something to do with #234 using the master branch and now using another branch?
You can try merging the current master branch into the local.

@wey-gu
Copy link
Member

wey-gu commented Oct 24, 2023

Let me give a try on resolving it.

@wey-gu
Copy link
Member

wey-gu commented Oct 24, 2023

@shbone Could you double-check check I didn't miss any changes of yours here?

if it looks good, we are good to merge.

I guess the conflict was related to the new line character introduced in a windows env.

@shbone
Copy link
Contributor Author

shbone commented Oct 24, 2023

Yes, I have checked it and all is right

@wey-gu wey-gu merged commit 70abae2 into nebula-contrib:master Oct 25, 2023
1 check passed
@wey-gu
Copy link
Member

wey-gu commented Oct 25, 2023

Let's merge then! :D

Thanks a lot for the great contribution!!!

@shbone shbone deleted the shb_dev2 branch November 26, 2023 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants