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/59 case variables #60

Merged
merged 12 commits into from
Jul 26, 2020
Merged

Feature/59 case variables #60

merged 12 commits into from
Jul 26, 2020

Conversation

jangalinski
Copy link
Contributor

@jangalinski jangalinski commented Jul 21, 2020

Adds support for variable access on caseService.

see #59

  • implement factories
  • implement adapters
  • finalize tests
  • add documentation

@jangalinski jangalinski self-assigned this Jul 21, 2020
@codecov
Copy link

codecov bot commented Jul 22, 2020

Codecov Report

Merging #60 into develop will decrease coverage by 1.21%.
The diff coverage is 53.17%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop      #60      +/-   ##
=============================================
- Coverage      77.59%   76.37%   -1.22%     
- Complexity       373      414      +41     
=============================================
  Files             56       62       +6     
  Lines            839      961     +122     
  Branches          72       78       +6     
=============================================
+ Hits             651      734      +83     
- Misses           140      179      +39     
  Partials          48       48              
Impacted Files Coverage Δ Complexity Δ
...a/adapter/basic/ReadWriteAdapterVariableScope.java 100.00% <ø> (ø) 7.00 <0.00> (ø)
.../adapter/list/ListReadWriteAdapterCaseService.java 28.57% <28.57%> (ø) 1.00 <1.00> (?)
...ta/adapter/map/MapReadWriteAdapterCaseService.java 28.57% <28.57%> (ø) 1.00 <1.00> (?)
...ta/adapter/set/SetReadWriteAdapterCaseService.java 28.57% <28.57%> (ø) 1.00 <1.00> (?)
...camunda/bpm/data/factory/BasicVariableFactory.java 69.09% <31.81%> (+12.33%) 22.00 <3.00> (+5.00)
...nda/bpm/data/reader/CaseServiceVariableReader.java 50.00% <50.00%> (ø) 5.00 <5.00> (?)
...nda/bpm/data/writer/CaseServiceVariableWriter.java 75.00% <75.00%> (ø) 10.00 <10.00> (?)
...va/io/holunda/camunda/bpm/data/CamundaBpmData.java 100.00% <100.00%> (ø) 22.00 <2.00> (+2.00)
...ata/adapter/basic/ReadWriteAdapterCaseService.java 100.00% <100.00%> (ø) 7.00 <7.00> (?)
.../camunda/bpm/data/factory/ListVariableFactory.java 92.00% <100.00%> (+9.39%) 19.00 <2.00> (+3.00)
... and 10 more

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 b336ba1...03d9751. Read the comment docs.

@jangalinski
Copy link
Contributor Author

@zambrovski If I do not hear objections, I will merge and release tomorrow ... need this feature at work asap.

@zambrovski
Copy link
Member

zambrovski commented Jul 24, 2020 via email

@zambrovski
Copy link
Member

zambrovski commented Jul 25, 2020

Bad idea to combine change of formatting (actually why was is needed?) with functional features.

It looks ok to me, the continuous decrease of coverage is kind of weak, but it is not solely the problem of your change. Still, we started with above 80% and are below 60% now... more complexity and less tests seems to be a risky path.

I could not find any examples how to use it... I think there should be examples for usage and for mocking inside the official docs... three lines inside the release docs / features might be not enough to describe the feature you implemented.

@jangalinski
Copy link
Contributor Author

Bad idea to combine change of formatting (actually why was is needed?) with functional features.

No idea why this wasn't formatted correctly in the first place ... should have been handled by the editorconfig

no examples ... cmmn is an edge case ... might add some later

@zambrovski
Copy link
Member

Hm. The editor config says 2 spaces not 4. Are you sure you applied the correct formatter?

@jangalinski
Copy link
Contributor Author

will check ... can you just fix it?

@zambrovski
Copy link
Member

No. I have a cellphone only.

@jangalinski
Copy link
Contributor Author

ah. :-)

ok ... will do

@jangalinski
Copy link
Contributor Author

will do reformatting in separated pr ... to much clutter

@zambrovski
Copy link
Member

zambrovski commented Jul 26, 2020 via email

@jangalinski
Copy link
Contributor Author

I asked you for one some time ago if you remember

yes, i know. see #61 but combined formatting of java/kotlin and formatting that matches codacy are still unsolved issues.

I will merge this and do a follow-up push with correct formatting

@jangalinski jangalinski merged commit a74fa8e into develop Jul 26, 2020
@jangalinski jangalinski deleted the feature/59_case_variables branch July 26, 2020 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
State: in progress Assignee is working on the issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants