-
Notifications
You must be signed in to change notification settings - Fork 388
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
Enhancement to add new options of diagonals and antidiagonals #996 #1224
Enhancement to add new options of diagonals and antidiagonals #996 #1224
Conversation
Please use one PR for background images and a second PR for the diagonals. |
The pull request #1224 includes only the diagonal changes. |
Hey Thomas, there is a hint. Just scroll down to the last page of the build log. Our build is crazy. It builds for over one hour. The problem is below, and I have restarted the workflow.
|
Thanks Wim for your restart, but something seems to be strange because the build failed again due to the same issue. I'm not a maven build expert, the only think I see is that the error message have an additional ":jar:" at detail message "dom4j:dom4j:jar:1.6.1", would it be possible that the repo has a changed path (I think this would be very strange) or is there an to do on our path/dependency...? |
I'm preparing for the 4.14 release. There I also updated the dependency version numbers, which might automagically fix this. @claesrosell @ruspl-afed are we caching the workspace? Could it be a problem in the maven cache? |
Try updating Tycho and downgrading Maven. eclipse/chemclipse#1331 |
I have done your mentiondes changes on my local side (pom.xml & yaml). Therefore I haven't checked in. Like a newbie with yaml I wil be very carefull with these changes. |
I have downgraded maven for now in master. I am currently working on the 4.14 releng and as a part of this I will upgrade to the latest and the greatest dependencies. I think the build will use whatever is in master. If not then please update your branch with the latest from master. |
Deleted the previous comment. The correct commit is: fa05ecd |
We have a master build running here: https://github.com/eclipse/birt/actions/runs/4372471029/jobs/7649432283 |
@wimjongman thanks for your hint. I updated my dev branch from your master. |
@wimjongman Have you perhaps an idea to solve the issues with the failed "model tests"? The build of this week of the latest pull request was sucessful, therefore I didn't understand the error messgaes because without my changes there is no difference from master-branch. |
I have looked in the logs, and there is one test failure:
It looks related to the stuff that you are doing. You can run this test locally and see why it fails. |
@wimjongman Thanks and I will take a look. This is also the reason why I haven't use the test framing active due to the big dependency issues on my local side. |
verify whether the simple standard reports are correct
You have to set the target platform with the birt.target file and run with the model launch configuration. They are in the birt.target plugin. Please take a look at the README.MD. All is described in a video: https://www.youtube.com/watch?v=FqfrG2I0AIw |
Your first hint was helpfull because the test-class used a wrong setting due to my changes - test is fixed. It is very confusing the the problems. I will take a look into your link and README.MD and hope this will help me for local testing. |
Sure! |
You can also post a discussion question so that others can benefit as well. |
The issues are solved. @wimjongman I used your instructions since I started but I saw on your video the small hint of unit tests and dependencies. The reason of my issue was thet the dependend PlugIns wasn't marked therefore the data are missing. The issues on the test clases based on 2 changed property values based on my change Currently the build finished in full also with the test cases. |
Great!
That is strange. why would they not be loaded correctly? I see that you have caught the issue. Does this hide the test failure? |
Yes, the new check of the null object avoid the issue. The message "invalid xml file" is correct if a null-object is casted to xml. |
What are our next steps here? |
@wimjongman Hello Wim do you see a way to review the changes of the new feature because it has an impact of further changes based on the implementation dependencies (no library dependencies). Thanks in advance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, Thomas! It looks good to me.
Thanks for your verification and merge! |
@speckyspooky I noticed two issues:
Regarding 2), I took the time to fix this in my branch; Please take a look at the following files in my In directory engine/org.eclipse.birt.report.engine/src/org/eclipse/birt/report/engine/css/engine:
The changes cost my several hours of work yesterday: I noticed in the debugger that for the new options, an index -1 was returned, so this resulted in an ArrayIndexOutOfBounds exception when trying to call the Spudsoft emitter. So it was clear that either the PerfectHash class had to be fixed or the whole PerfectHash thing should be removed from the code. Since the source code contained many magic constants, it was clear to me that the code was generated somehow. I was disappointed that even newer versions of the tool can't generate Java source directly, but soon I noticed the files token.* in the directory. I then recreated So I copy-pasted the constants, wordlist etc. from To my disappointment, the Excel emitter crashed instantly, it was totally messing up the properties. With further debugging, I noticed that the integer values inside Maybe in the future it would make sense to create a little script which magically fixes the numbers in |
@hvbtup The first result seems to me ok because I got no issue like you with the merge - here the result: You have written that you use another version of the SpudsoftEmitter like the BIRT-master-branch. Therefore I don't know whether I can use your changes because you have another BIRT-version based on your branch. Can you please egt a link to your commited file changes on your branch because in your previous link I didn't figured out the commit of your changes. But at the end it seems to me that I cannot copy your changed files directly because the dependencies of your branch in difference to the master. Your branch is many commits ahead of the BIRT-master-branch. |
Don't be shocked. You can in fact copy the four files I mentioned verbatim from the tup_main branch. They are not depending on any of the other changes in the branch.
I tried to do this more or less in my previous comment. The basic idea is like this: Input: So Then you need to have a working version of the GNU On the command line, inside the directory containing
So, The next step is to copy texts from The last (and tedious) step is to make the integer values in |
Thanks for your information. And thanks again for your details of PerfectHash. I took a deeper look into your notes and files so I can do it at the next time. |
Probably it makes sense to include the instructions as a |
…and antidiagonal #1224 (#1264) * renewing of the PerfectHash values due to the new properties of diagonal and antidiagonal #1224 * Update token.gperf Fixed incorrect comment about gperf command line options * README.md file of PerfectHash and new launch file of all engine tests * Update of README.md * Update README.md --------- Co-authored-by: Henning von Bargen <h.vonbargen@t-p.com>
of type assignments (eclipse-birt#1224)
Enhancement of diagonal lines - diagonal & antidigonal line
ROM added antidiagonal, properties available in the designer at page advanced properties
PDF added antidiagonal with color
diagonal line implementation: PDF diagonal border lines
FIEXED: fixed issue with cell background & diagonal line at the same time
HTML added digonal and antidiagonal lines with color
diagonal line implementation: png image
FIEXED: fixed issue with cell background & diagonal line at the same time
PPTX added antidiagonal with color
diagonal line implementation: PPTX diagonal border lines
DOCX added diagonal and antidiagonal with color
diagonal line implementation: DOCX diagonal border lines
XLSX added diagonal and antidiagonal with color
diagonal line implementation: XLSX diagonal border lines
new implementation of diagonal methode because the POI apache api doesn't support directly digonal lines
DOC added diagonal and antidiagonal with color
diagonal line implementation: DOC diagonal border lines
PPT proritary Actuate format, none Microsoft PPT format
added diagonal and antidiagonal
diagonal line implementation: DOC diagonal border lines
XLS unsupported
ODP added antidiagonal (no color support)
diagonal line implementation: ODP diagonal draw line
ODS unsupported
ODT unsupported
Documentation:
Class changes:
996-Change-Class-Changes.xlsx
Test reports:
diagonal_test_case_01_default.zip
Test results (output):
test_documents.zip