Re: Adopting Github Workflow
Michael Brohl writes: > Am 11.03.20 um 19:13 schrieb Mathieu Lirzin: > >> So to “manage” stuff you could still use tickets and ML discussion like >> before, but it would just be done when necesarry not to follow the >> “everything as to be attached to JIRA” policy which was often justified >> by the fact that they serves in the Monthly reports. > > Do you speak of the monthly blog posts? Yes, > They are solely derived from the Git commit history, Jira is not used > for it. I was unaware of that. This is interesting because now I am realizing that all the arguments I heard in the past about the “the need to create a JIRA ticket for each meaningful code contribution because otherwise it will not appear in the Monthly blog posts” was even more bureaucratic non-sense… I will not response in details to the others points you made, because I am now convinced that we simply leave in alternate realities and that there is no way we can possibly have a constructive discussion together. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Adopting Github Workflow
Jacques Le Roux writes: > Le 11/03/2020 à 17:08, Michael Brohl a écrit : >> >> Am 11.03.20 um 16:29 schrieb Mathieu Lirzin: >>> - Adopt Github Pull Request (PR) as the unique channel for code contribution >> >> -1 >> >> I don't see a reason why we should not allow patches also. It will >> make it easier for people to contribute who are not able (or >> willing) to run their own GitHub repository. >> >> We should encourage people to use PR's but not force it (at least, not >> immediately...). > > I agree with Michael, using only GitHub (GH) is too radical, IMO even in the > future. > > Also the Jacopo's remark about using (only?) a 3rd party tool should > be checked if ever we get this way. Jira is also a 3rd party tool but > it's donated by Atlassian to the ASF and handled/customised/maintained > by the Infra team. Also it uses a customised version of the OFBiz > Entity Engine, own dog food? :) > [...] > +1 with Michael's comment, Jira is very handy when it comes to details > like uploading info, etc. Not sure GH provides the same > If seasoned PMC members like you or Michael are not ready to give up on considering JIRA as a central tool to “manage” every code contribution the discussion is over on my side. Adopting the Github Workflow ontop of an already indigest cake of contribution requirements can only make things worse. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Adopting Github Workflow
Mathieu Lirzin writes: > The reason I see for requiring a unique code contribution channel is > lowering cognitive overload for everyone. > > If a community ends up “requiring” certain contribution procedures that ^^^ then > people are not forced to conform. It is then up to the reviewers to ^^^ should be > adapt or not to the choices made by the contributor. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Adopting Github Workflow
Hello Michael, Michael Brohl writes: > Am 11.03.20 um 16:29 schrieb Mathieu Lirzin: >> You are right. I should be more constructive than just acknowledging >> that the requirements I expressed were not properly considered. >> >> Let me try joining Pierre's effort by providing a simple but radical >> proposal for our contribution procedure : >> >> - Adopt Github Pull Request (PR) as the unique channel for code contribution > > -1 > > I don't see a reason why we should not allow patches also. It will > make it easier for people to contribute who are not able (or willing) > to run their own GitHub repository. > > We should encourage people to use PR's but not force it (at least, not > immediately...). The reason I see for requiring a unique code contribution channel is lowering cognitive overload for everyone. If a community ends up “requiring” certain contribution procedures that people are not forced to conform. It is then up to the reviewers to adapt or not to the choices made by the contributor. If you are willing to adapt and continue review patches attached to a ticket in parallel of the PR then those contributors will be happy. >> - Require tickets only for bug reports > > -1 > > Jira tickets for improvements/new features are extremely helpful to > have everything in one place (with the addition to a link to the > mailing list discussion), track the efforts, watch issues etc. > > How would you manage the work then? I think you misinterpreted the proposition, it is not because something is not required for every code contribution that it cannot happen when it matters, indeed in the case of long term efforts like for instance the “Groovy migration” it would make sense to have an issue associated with it, but for small code/documentation improvements having a ticket serves only a review medium which is far inferior than Github PR mechanism. So to “manage” stuff you could still use tickets and ML discussion like before, but it would just be done when necesarry not to follow the “everything as to be attached to JIRA” policy which was often justified by the fact that they serves in the Monthly reports. >> - Replace JIRA with Github issues > > -1 > > Why should we have two different issue management systems when we can > have everything in one place? What advantages do you see for doing so? > I also would not rely too heavily on GitHub because it is not an > official ASF resource and IMO Jira is a huge knowledge base of the > project. The advantage with Github issues is that they are well integrated with the Pull Request mechanism so it is easier to reference Github issues in PR (the reverse is also true). IME Jira does not help avoiding the “Cognitive overload” because its features are thought for the enteprise context with time tracked employee and managers producing reports to their superior which is far too burdensome to work with in the context of a community project. AFAIK the “everything in one place” you talk about is mostly fantasized and has never existed IME. People often have to cross reference parallel discussion happening on Jira and Mailing list. The adoption of Github PR in the contribution framework I am proposing will not improve that issue but at least it will not make it worse as it would be if OFBiz adopts Github PR as “yet another” contribution option ontop of existing ones. >> - Use Github API to get the stats for OFBiz monthly reports > > Can you elaborate what you mean/ how you would do it? I don't know and I don't want to know ;-). I am just aware that Github has a Web API which enables the retrieval of statistics on Issues/PR/Contributors/... which can be useful for satisfying the requirement of assisting the creation of monthly reports which some people in this community care about (full disclosure I don't). -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Git history problem
Jacques Le Roux writes: > Le 11/03/2020 à 12:33, Mathieu Lirzin a écrit : >> Yes things seems to happen not magically but by putting earplugs on and >> going ahead, which is certainly more effective but IMO not acceptable >> when working within a community. > > I'm not sure to who it's destined, so I'll not take it for me :) This was destined to no-one, it was just general remark regarding handling community input on a proposal. So you do not have to feel targeted. By the way the expression “not acceptable” was probably too strong and should have been “not a good thing” because as Pierre said this is eventually necessary to get things done. >> The simple solution to prevent this is to get into the habit of >> linearizing history, meaning always rebasing and clean history before >> merging into trunk. > > I guess the GH merge button option "Rebase and merge" is what we are > looking to enforce with the request to Infra, right? I personnally think we should have *no button* because the committers should cleanup the commits (reword, squash, ...) before merging to trunk, but "rebase and merge" is an improvement compared to basic "merge". -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Adopting Github Workflow (was: Git history problem)
Jacques Le Roux writes: > Le 11/03/2020 à 12:33, Mathieu Lirzin a écrit : >>>> This said you certainly saw this thread started by Pierre Smits: >>> https://markmail.org/message/so7ljoqxzuq7jplz and the related wiki >>> document >>> https://cwiki.apache.org/confluence/display/OFBIZ/Contributing+via+Git+and+Github >> AIUI this page is only collection of Git/Github tips and fuzzy >> "maybe"/"you can" rules. Moreover it does not propose to >> replace/deprecate/simplify existing contribution procedures/tools. > > Yes, that's really missing, I agree. I once created OFBIZ-1129 (Tools to > backport), but finally closed it for 2 reasons: > > 1. Not enough interest (most important reason by far) > 2. Spent some time, did not find easy way to replace scripts, I don't think > fits. > > I think Pierre's effort is a try for the procedure aspect. Why not > joining there and improve? You are right. I should be more constructive than just acknowledging that the requirements I expressed were not properly considered. Let me try joining Pierre's effort by providing a simple but radical proposal for our contribution procedure : - Adopt Github Pull Request (PR) as the unique channel for code contribution - Require tickets only for bug reports - Replace JIRA with Github issues - Use Github API to get the stats for OFBiz monthly reports @Pierre: What do you think ? -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Git history problem
Hello Jacques, Jacques Le Roux writes: > If my opinion is worth telling, I was initially reluctant to use the > PR process instead of the patch process. Because in general I prefer > to control things, it's even sometimes a problem for me in real > life. I must say it was more laziness which inclined me to PR. Your opinion is always worth telling on this topic since to my knowledge you are the most active reviewer which really valuable. > Like Michael I had a look about the settings. The settings button is > available on my fork but not on the official mirror Github repo. Thanks for double checking. >> This said you certainly saw this thread started by Pierre Smits: > https://markmail.org/message/so7ljoqxzuq7jplz and the related wiki > document > https://cwiki.apache.org/confluence/display/OFBIZ/Contributing+via+Git+and+Github AIUI this page is only collection of Git/Github tips and fuzzy "maybe"/"you can" rules. Moreover it does not propose to replace/deprecate/simplify existing contribution procedures/tools. So it misses the point that adopting Github workflow only makes sense if it simplifies the contribution process for both contributors and reviewers. > The discussion ended with Michael's disagreement, as somehow yours, > but nothing happened. So I think we should move ahead with your > proposition and note the change in the wiki page. I have created > https://issues.apache.org/jira/browse/INFRA-19950 for that. If > somebody disagrees please speak now, even if it will always possible > to revert the change. > > We have also to maintain the related stuff which are also still WIP: > > https://cwiki.apache.org/confluence/display/OFBIZ/Migration+from+Subversion+%28SVN%29+to+Git > https://issues.apache.org/jira/browse/OFBIZ-11268 > > Nothing happens magically, but it seems we are near the end about the > migration from Subversion to Git process. Yes things seems to happen not magically but by putting earplugs on and going ahead, which is certainly more effective but IMO not acceptable when working within a community. > BTW, a question for you: do you think not having a linear commit > history would have an impact on Bisect. I don't think so, just asking > in case you have an experience with that. ‘git bisect’ handles non-linear history nicely but it achieves this with a more complex algorithm than basic binary search [1][2]. The difficulty is when a commit identified as introducing the regression is a merge commit because it requires analysing each branch up to their common ancestor. The major issue is more social, because once people adopts the practice of merging branches without rebasing and cleaning that branch first, since contributors often branch from another development branch, this will eventually lead to have the same commit present multiple times but with different hashes. The simple solution to prevent this is to get into the habit of linearizing history, meaning always rebasing and clean history before merging into trunk. > Please don't be angry, it's not good for health. Just listen few > minutes to https://www.youtube.com/watch?v=d-diB65scQU it generally > helps ;) :-) [1] https://mirrors.edge.kernel.org/pub/software/scm/git/docs/git-bisect-lk2009.html#_git_bisect_details [2] https://www.wikipedia.org/wiki/Binary_search_algorithm -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Git commits email notification problem (was: Git history problem)
Hello Jacques, Here is a first answer on the specific point of the commit notification issue. Jacques Le Roux writes: > I noticed that sometimes strange things happen when you use a > PR. Consider this recent email for instance: > https://markmail.org/message/amq5fj2dfma7pcbb. > > There is only the files names, nothing about the changes. You have to > get there to have the information > https://github.com/apache/ofbiz-plugins/commit/4afe603/ not very > convenient :/ > > Moreover if you look at the commit comment in related Jira (OFBIZ-10948), it > says: > >< <https://issues.apache.org/jira/browse/OFBIZ-10948> > >Improved: Convert DimensionServices.xml minilang to groovy (OFBIZ-10948 > <https://issues.apache.org/jira/browse/OFBIZ-10948>)>> > > When I supposed it would be only the title and comment at > https://github.com/apache/ofbiz-plugins/pull/7. Why is "Merge pull > request #7 from priyasharma1/OFBIZ-10948 > <https://issues.apache.org/jira/browse/OFBIZ-10948>" added, mystery! > > For a reason I ignore (must be a reason, but I'll not try to understand) we > did not get that. I guess this is related to the synchronization between Gitbox and Github. because that email says that 4087 revisions have been added which correspond to the total number of commits in the ofbiz-plugins repository. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Git history problem
Michael Brohl writes: > I tried to follow the instructions found behind the link Mathieu > provided and see that I cannot change the project settings. > > Who has access to the project settings and can grant me permission to > change settings? Thanks Michael for this attempt. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Git history problem
Hello, The history of OFBiz trunk with the adoption of the Pull Request based contribution process is getting less and less readable. Here is a snippet of `git log --oneline --graph` demonstrating that: --8<---cut here---start->8--- a3bcdc4cc3 * | Improved: Removes getSubContentWithPermCheck and getSubSub |/ 8ec3886d7f * Fixed: Code refactoring to support groovy syntax (OFBIZ-1023 604a3bfa02 * Improved: Convert PartyInvitationService.xml from minilang t 5b8c89906c * Merge pull request #36 from danwatford/ofbiz-11428-checkst |\ e665f9dc68 | * Improved: Set checkstyle to use LF line endings e9ec4181ec * | Merge pull request #17 from PierreSmits/Label-Cleanup |\ \ 974b85f4ec | * | Improved: Cleanup HumanRes labels c71a7ae06d | * | Improved: UI labels c121ad6b9d * | | Merge pull request #15 from PierreSmits/OFBIZ-10551 |\ \ \ | |_|/ |/| | 58b0da26f5 | * | Improved: Remove unused labels from ProductUiLabels.xml 66aa76d7f7 * | | Merge pull request #35 from danwatford/ofbiz-11418-doc |\ \ \ 0ece441228 | * | | Fixed: Fixed line lengths in ModelFormFieldTest to adh cfad407c48 * | | | Merge pull request #34 from danwatford/ofbiz-11418-d |\ \ \ \ | |/ / / 5640de4eba | * | | Documented: Documented use of field attribute paramete --8<---cut here---end--->8--- I personnally think this is a huge issue because it makes analysing history and chasing previously introduced bugs unecessary hard. I would strongly recommend configuring OFBiz Github to require a linear commit history when merging PR [1]. Thanks. [1] https://help.github.com/en/github/administering-a-repository/requiring-a-linear-commit-history PS: Even if I try to be polite, I am profoundly angry regarding the way the PR contribution process has been adopted without any formal community approval or listening to people having stated important requirements that needed to be addressed before moving towards that new contribution process. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: OFBIZ-4035: change the id Attribute for input fields in the model macro form renderer from String to FlexibleStringExpander
Daniel Watford writes: > We already have a +1 from James. Can I count on your vote, Mathieu? :) You have a +0 on my side. As I already said I support the testing effort because OFBiz need people caring about writing maintainable tests and I found legitimate to introduce an extra mocking library dependency to mitigate the poor testability of current implementation even if it is not ideal. > Regarding the example argument “JMockit is redundant with Mockito purpose > which is already used”: > I hope I've demonstrated in this thread that Mockito is not fit for purpose > in this particular case due to the architecture of the MacroFormRenderer > class; and that refactoring MacroFormRenderer without the support of tests > is risky and therefore a worse proposition that introducing JMockit. > Therefore a veto with that particular argument shouldn't be considered > valid. I don't think this demonstration can effectively prove that this hypothetical technical argument is invalid because “worse” depends on the perspective and software is always about trade-offs meaning that something that is “worse” in some facet is “better” in another. Imagine a potential PMC member who cares more about the size of the artifact footprint than about the actual test coverage. With a bit of insistance combined with a dose of FUD [1] (...customers will drop OFBiz, ...this will introduce dependency incompatibilities) she can easily make it appear as a valid “technical” argument to some doubtful people. So the best you can do is to demonstrate that something is more desirable than some alternative trade-offs with the limit that you have a finite time to spend on that thing and that people can have infinitely many alternatives in mind. Which mean concretely in the presence of stubborn or dishonest people no way to demonstrate anything. ;-) This is going off-topic and since nobody is currently making such kind of arguments so let's drop this discussion. :-) Thanks. [1] https://en.wikipedia.org/wiki/Fear%2C_uncertainty%2C_and_doubt -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: OFBIZ-4035: change the id Attribute for input fields in the model macro form renderer from String to FlexibleStringExpander
Daniel Watford writes: > Yes I agree that mocking of static methods is an indicator of refactoring > needed. > > There are always exceptions to every rule, but in this case with > MacroFormRenderer longer than > 3000 lines in R18 and its role is to map from ModelFormField instances to > string representation > of FTL macro calls, and then to execute those FTL macros, it feels like an > excellent candidate for a > bit of refactoring. I feel the pain. > However, to do so without any sort of test framework in place will be > introducing risk. You are right. If you are fearful of the code you touch (which is legitimate in this case) indeed adding some test automation is a nice idea. > At first a refactoring effort might seem simple, but with such a large > class we are likely to find some difficulties. Therefore if I had a > vote I would suggest accepting the JMockit based tests AFAIC I think the unbearable complexity and awfulness of the implementation FTL/Screen rendering justifies adopting complex testing techniques and adding a new even partially redundant dependency like JMockit, even if this is sad to have to do so. PS: Votes in regards code modification in a community with PMC members that basically disagree on everything else than “We are an awesome community” is not a solution to anything because any veto with a valid technical decision like for example “JMockit is redundant with Mockito purpose which is already used” would simply block your proposition, so a vote for code modification serves only to validate a consensus. [1] [1] https://www.apache.org/foundation/voting.html#votes-on-code-modification > with a view to retire and replace them with more standard and > recognisable Mockito tests once made possible through refactoring. I would suggest seeking the goal of adopting basic dependency injection with explicit method/constructor arguments which leads to simpler tests and is a sign of well designed code, instead of seeking the replacement of advanced mocking techniques with less advanced ones. Mocking is sometimes useful but should be avoided when possible. > If the committers accept https://github.com/apache/ofbiz-framework/pull/26 > and > https://github.com/apache/ofbiz-framework/pull/27 I will be happy to open > and work on a ticket to > refactor MacroFormRenderer and associated tests. I'd aim to deliver any > refactoring as a number > of small patches/PRs over a period of time. Good luck. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: OFBIZ-4035: change the id Attribute for input fields in the model macro form renderer from String to FlexibleStringExpander
Hello Daniel, Daniel Watford writes: > I don't have a preference between the two mocking libraries, but creating > unit tests for MacroFormRenderer necessitates mocking static methods. > Mockito doesn't offer static method mocking, but JMockit does. > [...] > To provide unit tests for MacroFormRenderer without using a mocking library > like JMockit will require the class to be refactored which I deemed a bit > of a risky change at this point, particularly without existing tests to > prove correct behaviour is maintained. Any effort that take into account how things can effectively be tested is a good thing for OFBiz. :-) My impression is that the "need" to mock static methods is a symptom of entangled code. And that making the code simpler and better designed will remove that need. As you said having to refactor without tests in the first place is not good but usage of advanced mocking techniques to workaround the fact that basic unit testing is too hard is not ideal neither. Another solution (that is not ideal neither) is to rely on manual testing in the first place (or automatic integration tests if you have time) introduce the architectural refactoring and then add you unit test with explicit arguments based dependency injection instead of mocks. In any case that is a matter of compromise. :-) -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Allow shutdown in Gradle without building the project (Allow shutdown in Gradle without building the project)
Mathieu Lirzin writes: > If people want to be in charge of the compilation and execution > lifecycles, they are free to use Gradle to build an artifact and run > OFBiz outside of their build tool like in the following example: > > ``` > $ ./gradlew jar > $ java -jar build/libs/ofbiz & > $ java -jar build/libs/ofbiz --shutDown > ``` Oups, I forgot the “.jar” extension and misspelled --shutdown. Here are the correct commands. ``` $ ./gradlew jar $ java -jar build/libs/ofbiz.jar & $ java -jar build/libs/ofbiz.jar --shutdown ``` -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Allow shutdown in Gradle without building the project (Allow shutdown in Gradle without building the project)
Hello, Taher Alkhateeb writes: > I think I understand the reason why this is happening. Someone changed > the implementation of createOfbizCommandTask to depend on the classes > directly instead of depending on the generated Jar. I'm not sure why > but I personally do not prefer this approach. > > The solution is to revert to the original design where the classpath = > files(thejarfilehere) and change dependsOn from classes to build Running OFBiz via Gradle only depends on building the classes because it does not need more. Depending on stuff like :jar or :build will only slow down the recompilation process for no sound benefits. Additionally this approach matches what is done for the standard :run task. The whole idea of running a program from Gradle which is a *build* tool is to make sure that the executable is in sync with the current state of the sources which is important in a development context. As a consequence the idea of disabling this fundamental feature in presence of the --shutdown option simply does not make sense. If people want to be in charge of the compilation and execution lifecycles, they are free to use Gradle to build an artifact and run OFBiz outside of their build tool like in the following example: ``` $ ./gradlew jar $ java -jar build/libs/ofbiz & $ java -jar build/libs/ofbiz --shutDown ``` I will not try to argue with people that this is the right approach so feel free to ignore my input if it does not make sense. In any case you have my blessing for reverting what “someone” sneaked in the code base. ;-) Regards, -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
PMC/Committer resignation
Hello, As of today, I am resigning from my PMC and Committer role. My initial involvement with OFBiz started in April 2018 as an internship in the Néréide team to work on the support RESTful Web APIs. While working on that topic I discovered soon enough that this objective would take a very long time to achieve because of the huge technical debt that has grown over time in the framework internals. Moreover I have observed that my colleagues were swamped in the accidental complexity of OFBiz (polyglot programming model, cumbersome DSLs, weak typing, ...) which is impacting negatively their ability to deliver value to their client. Following those observations, I have spent a huge amount of my time (including my weekends) trying to understand, remove, refactor existing code to improve OFBiz simplicity which is a pre-requisite for staying relevant in the future. The recent events/discussions lead me to the conclusion that my efforts towards the goal of simplifying OFBiz will simply not be able to succeed due of the required slow review process and the global lack of interest/consensus on that topic. As a consequence I am unable to continue to participate positively in this community anymore. Kind regards, -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Hello Taher, Taher Alkhateeb writes: > I can sense frustration from Mathieu regarding getting things moving > forward. You are correct. > I would just like to note that it's really not _that_ bad! You've > already gotten a lot of commits rolling into the code base (which is > fantastic) and you didn't get objections on most of them. In fact many > commits that you made were just fine, including things you did to the > core components and gradle scripts and whatnot. Yes I am glad about that. However such achievement has been possible only because I chosed to move fast for changes I considered as implicit implementation details. Apparently such practice do not follow the recommended guidelines which require a code change proposal to be slowly reviewed first even for trivial stuff because we can never know if it might break user code or not. > So I want to encourage you to trust that things would work themselves > out, and there is no need to be frustrated. Take a deep breath and > just consider more of a long-term initiative than a short term one. > Doing the jar architecture is not the only thing that can be improved. > There is a whole heap of areas in the framework that can be improved. > For example REST (which you worked on partially if I'm not mistaken). The global goal is not just to improve various areas for the sake of improving them. The objective is to end up with a piece of software that is useable, extensible and works reliably. I tried my best to implement the REST stuff (and Artemiy after me) without going as far as I intended because I faced the immense complexity of ‘RequestHandler#doRequest’ which involves complex mechanisms like “override view URI”, ad-hoc redirection, messy error handling, HTTP query parameter passed in the path with a ‘~’ prefix which are used in templated links inside Freemarker code... all that paired with the unbearable complexity of the screen DSL to implement a JSON view handler... Clearly I am not convinced that following that approach will eventually turn into something useable and reliable in the long run. Thank you anyway Taher for trying to find something positive within a failure. :-) -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Michael Brohl writes: > I think depends-on is a point we already have discussed and this was > not my topic in the latest emails. My proposal to write up a concept > is adressed to the "big picture" you have described in [1], which > contains your statement: > > "Here is a *big* change that I am considering for OFBiz to fixes those > issues by leveraging the Java platform which already provides > everything that we need to fix those issues: ..." > > I was talking about this big change and the plan behind it. In the > initial discussion you gave a brief vision, which should be worked out > by the community to move forward. A vision is far from a concept the > community can decide on, which is my main point I expressed earlier. If we failed to understand each other on the small picture, I doubt bringing the “big picture” will be lead to better result. The bigger the scope is the more likely it will end up in a “what if” tar pit. >> I don't see the point of continuing this unproductive discussion neither >> to proceed with a formal vote regarding the deprecation of >> “component-load.xml”, because whatever the result I have lost my faith >> in the capability of this community to succeed at handling the technical >> challenges that will enable OFBiz to stay relevant in the future. >> >> But this is fine. > > As I've sorted out the "depends-on" topic as the reason for the wish > for a concept/plan: do you also think that a discussion about the *big > change* is unproductive and is not necessary? Maybe a few month ago I would have been more patient and open to get into the requirement analysis details, but now that I have already spent all my energy into related heated debates without having any time left on realizing what I intended to work on initially, I am basically done. It could have been productive but in an alternate reality. > How do you do conceptual work with clients or colleagues? I believe > there is some kind of written documentation and clear decision points > involved at least for non trivial features/changes. Usually such discussion involves a whiteboard and a face-to-face discussion. Nereide has not a strong culture of written specifications and work in a very agile way. > I sincerely hope that we can sort out the resentments and find a way > to collaborate on the challenges that lay ahead. I am afraid that I am out of fuel here. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Hello,
Michael Brohl writes:
> Am 31.01.20 um 13:15 schrieb Jacques Le Roux:
>>
>> We could continue the discussion in this thread or at
>> https://issues.apache.org/jira/browse/OFBIZ-11296
>
>
> This issue shows exactly the same pattern in the process like the
> component-load approach. The Jira was created and *on the same day*
> the first patches were committed without further discussion within the
> Jira. The Jiras contains a link to a discussion with the statement
> "this has been discusses on Development mailing list".
>
> In fact, the thread also started on the same day in dev but has not
> many reactions and in no way any decision to go in that
> direction. This is by no means the correct way to introduce
> fundamental changes to the project.
This description is dishonest, I have always opened discussions on this
mailing-list and waited for feedback when considering a *big/breaking
change*. I have only moved fast when I considered that what I was
working on was an implementation detail and that the improvement was
obvious.
The actual issue is not that I did not follow the rules. The fact that I
ended up opening/closing a JIRA the same day I commit things that I
consider trivial was precisely to conform to the policy of “every change
needs to have a JIRA associated” which is bureaucratic non-sense.
The actual issue is that in the case of
“{framework,applications}/component-load.xml” we disagreed on the
trivial aspect of this change. This disagreement is an expected thing
because nobody in this community can tell the difference between an
*implementation detail* and a *breaking change* because we don't have
(and don't want) a public API. This simply means that every code change
is a breaking change and that it should follow a slow process of review
to let evaluate the trade-offs between potential downstream breakage
(that we might not be aware of) and the actual benefits of the proposed
change.
> I'm also pretty sure that, if the community decides to go that way
> after a thorough exchange of arguments and real life practical
> experience, the implementation will be a long-running project
> itself. This cannot be undertaken on the trunk but will need a feature
> branch, which has to be discussed when the time is right.
>
> As much as I appreciate the initiative to move things forward I am
> also strictly against the approach and process to put fundamental
> changes into the codebase without through conceptual work and
> planning.
I am glad that you appreciate the initiative, but as far as I am
concerned I am certainly not enjoying my time in this community anymore.
>> https://issues.apache.org/jira/browse/OFBIZ-11161 is also related
>>
>> Maybe better to create a new thread?
>
> We already have a thread for it, started on 22. August 2019. I would
> very much appreciate if experienced users/developers would join this
> discussion (which I have missed being on vacation at the time and,
> having not much response, did not get my attention until now).
Basically you are acknowledging that nobody cares deeply about the idea
I proposed in previous thread (which is probably true) but at the same
time you are suggesting me to write a lengthy design/architectural
document that will rephrase what as already been said in that thread.
Sorry but no, I will not write such document, I have already explained
multiple times the design principles leading to the proposal of enabling
the distribution of OFBiz inside a Jar:
- Extensible dependency management is better than having to define a
arbitrary global ordering
- Location independent loading/configuration mechanism is the only sane
way to provide true extensibility.
- Adopting the stable mechanism provided by the Java platform that we
already depend on is better than implementing our own specific
mechanism
If people are not convinced by the benefits of this vision which imply
deprecating the “component-load.xml” mechanism and use “depends-on”
instead (which I did not introduced in the first place) then providing a
precise design/implementation plan will not help moving forward, it will
just lead to more time waste on my side.
I don't see the point of continuing this unproductive discussion neither
to proceed with a formal vote regarding the deprecation of
“component-load.xml”, because whatever the result I have lost my faith
in the capability of this community to succeed at handling the technical
challenges that will enable OFBiz to stay relevant in the future.
But this is fine.
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: OFBiz contributions & Github Pull Requests
Michael Brohl writes: > I really don't want to see all kinds of exotic messages coming up in > our commit history. > > If the workflow allows rejecting a pull request because of bad commit > messages or maybe a hundred commits for a change, I would also be fine > with it. > > My main point is that I'm a bit worried about quality because it is > much easier to pull in a pull request instead of applying a patch. +1 I am worried too, Handling pull-requests via Github Web interface makes really easy to screw the commit history up. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Jacques Le Roux writes: >> What question have you in mind for the vote? > > The idea is to vote about removing component-load.xml from the > framework, as Michael agreed on, but let them in applications and > plugins. Is that OK with your plan? Not really but that is better than nothing. Should the vote question include the fact that “component-load.xml” feature will be deprecated in the next release and will be removed after one release cycle? -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Hello Jacques, Jacques Le Roux writes: > You were the most visible proponents of this. After 5 days w/o > exchanges should we consider the idea is abandoned or should we start > a vote? > > Without answers I'll consider the idea abandoned, which is a pity IMO. The idea is not abandonned yet, it is just that I need to take some time off to accept that my strategy consisting in trying to bring important changes within OFBiz might actually be quixotic. After re-reading [1] I am not sure to understand how a vote can help resolving this issue because any -1 for code modification is considered a veto (if it is “accompanied by a technical justification”). So unless the question that we vote on corresponds to Michael minimal requirements, it will not enable us to move forward. Am I overlooking something? What question have you in mind for the vote? Thanks for trying to make things move forward. [1] https://www.apache.org/foundation/voting.html#Veto -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: [ofbiz-framework] branch trunk updated (6f39741 -> 6d194cf)
Hello Jacques, "jler...@apache.org" writes: > This is in relation with "Github PRs and Jira" thread > (https://markmail.org/message/s422zuivbmzglph4) > > Below is my 1st experience of GitHub PR merge in the context of ASF dual > hosting with GitBox. So I somehow made a jump in the void... > > After reviewing the change I picked "squash and merge PR" among the 3 > GitHub PR merge options. Because I did not want to break the linear > Git flow, it seemed the best option. But despite "squashing" we got 3 > commits; 2 from Daniel Watford and one from I. I don't know if it's > possible to do otherwise with the 2 other options. Maybe it's good to > know that Daniel contributed... > > Also I expected that my comment in GitHub would be used as the final > commit comment. So I then tried to amend the commit by modifying ONLY > the comment, and then push comment after a pull. But I got a > fast-forward issue and after trying several pull and push tricks I > understood I'll never pass over this issue: > >"[remote rejected] trunk -> trunk (pre-receive hook declined)" > > Because it's related to having "receive.denynonfastforwards" set to > true on GitBox server[1] and I don't want to annoy Infra to temporary > set it true for a such change. One must never, never, never rewrite history of the trunk branch. :-) > But I really wanted to change the commit comment to follow our > template. So I decided to pull rebase, slightly change the > ComponentContainerTest class, commit and push. Hence I created a new > commit, somehow a duplicate of the PR merge, at least with a correctly > formatted comment. > > This was an awkward experience, I don't know if i could have done it better, > ideas, advices? > Maybe advising contributors to use our template in commit comments used by > GitHub PRs? The best option I think is to clone the branch locally, amend the necessary commits, push on the PR branch ask for validation from the original author, and then from your terminal rebase on trunk and fast-forward merge the commits (meaning without a merge commit). Basically the better option to use Github Web interface only for discussion/review. > Also we can ask our contributors to use in the PRs title the format of > our commit template. For instance in[2] instead of > >OFBIZ-11331: Allow ComponentContainerTest to run on windows >rather >Fixed: Allow ComponentContainerTest to run on windows (OFBIZ-11331) > >Then the commit/s would contain the explanations. > > I just fear we will have sometimes to explain, and ask contributors to edit > their comments or titles... > > I'll sleep on it and wait for comments before doing any additions or > changes in our documentation about that, in relation with the "Github > PRs and Jira" thread I think it would be better to not use the PR feature as long as there is not a community consensus that this is an acceptable contribution channel. As stated previously I am not against adopting Github PR *if and only if* we have a plan to move out of Jira. Given the pile of existing policies, I don't think this is a good idea to simply provide more contribution options/policies, I think it is important to review existing ones and consider replacing some of them. For example we could deprecate patch based contribution, or at least adopt the ‘git format-patch’ conventions. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: [VOTE] Do not release R17 and directly publish R18 instead.
Jacques Le Roux writes: > Vote: > [ +1] Do not release R17 and directly publish R18 instead. > [ -1] Release R17 before releasing R18 +1 -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: What is OFBiz public API?
Hello, Taher Alkhateeb writes: > While working on all the above, I never faced major obstacles. But the > reason that I did not is I always made a complete, well written > argument about what I'm trying to implement. I always ask for a pair > of eyes to look at my code and give me feedback, and I always engage > with the community. In my opinion, disagreements made me do some of > the best pieces of code because I learn and grow from other people's > input. That sounds a lot like a polite way to suggest that I did not engage properly with the community. I have done my best to get people involved while acknowledging the need to move relatively fast which IMO is justified by the abyssal technical debt of OFBiz. > The boundary of a public API does not necessarily need to be agreed > upon RIGHT NOW!. I don't think the best way to move forward is to > enforce agreeing on what we should done before hand. > > Instead of setting rules and boundaries on what should and should not > be done, I recommend instead collaborating on all work, one piece at a > time. As I said, I've worked on very large issues indeed and had no > problem working it with others and making it eventually to the code > base. > > So whenever there is a disagreement or differing points of view, why > not start a new thread specific to that topic and work it out. > Everybody wants the best for the project, and communication is the key > to moving us forward IMHO. I agree that communication is important as long as it enables people to move forward, currently we are blocked. If you look at the initial discussion, you will see that this is precisely a deadlock in a *thread specific* conversation between Michael and me that lead to this general question. This specific discussion is simply unsolvable because everybody is basing their arguments on undocumented assumptions regarding what needs to be preserved, what needs to be superseded, how things are supposed to be done, etc. This is basically a sterile “you broke my business specific extension” vs “I want to improve the framework” debate where both perspective can be seen as legitimate but are fundamentally incompatible with each other in practice. To recap the discussion, it appears that most people that have responded to my call for defining/documenting what is part of OFBiz public API feel this would be an unnecessary and counter-productive action and that case by case discussions can be a more valuable substitute. As a consequence I think we can close the subject now. Thanks to everyone who took some of their time to participate. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: What is OFBiz public API?
loper to help them. Olivier Heintz writes: > 4) implementation detail or core choice, often, it's depend of point > of view you use On ERP development / implementation there are multiple > type of people working on it with each one, specifics knowledge, > usage, practice and point of view. When someone says, it's a big > change, start by trusting him and find out which process is affected > to propose a solution; he doesn't want to block something, he wants to > help you come up with a better solution that solves more cases. Given that it is very easy to get a convincing argument by saying “people might depend on …”, I am rather skeptic. Even if I trust others for providing valuable inputs or proposing good ideas, I am confident that if someone happen to rely on any mechanism in production that is considered for removal/rewrite he/she will always choose a solution that make the person proposing the change to do whatever is necessary to not require them to do a migration work. Again I am not saying that this is wrong, I am just saying that this natural force should be balanced with something assessing that only a subset of all mechanisms are subjects to such argument and using the complement of that subset mean that users are on their own and they should be prepared for breaking changes. > 6) what is OFBiz, > - not only a public API ;-) > - not only a framework > - currently not a OOTB ERP but I hope what, in future, there will contain > some OOTB applications > In my long term view Apache OFBiz could be like linux, a core/kernel as small > as possible with multiple components (the plugins) and so with some > distributions. > So, clearly, for me it is not possible to define the boundary between what is > public (with backward compatibility) and what is only implementation. > Even on data model I can give some examples where not everyone will agree on > what is in kernel and what is in components I believe that any effort of modularizing the code base without relying on a stable public API like moving plugins inside independent repositories is doomed. I am personnally still not convince that seeking for dozens of custom plugins is a good idea even with a stable public API. Regarding the core + plugins ideal, it is interesting that you brought the linux kernel example because it is the perfect example of a monolithic development process where every driver/module (plugin) that seeks for backward compatibility must be maintained inside the main repository because there is no guarantee on the interface between the core and the plugins. A week ago, here is what Linus Torvalds was saying to people considering developing a module for ZFS (a file system) outside of the main repository [1]. I am curious about the data model example you have in mind? [1] https://www.realworldtech.com/forum/?threadid=189711&curpostid=189841 ¹ The question about “plugins/component-load.xml” is different because there is a consensus on the fact that is part of public API however I have the impression that it is still in alpha state given that plugins mechanism is relatively new. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Fwd: Re: What is OFBiz public API?
Hello Jacques, Jacques Le Roux writes: > Le 05/01/2020 à 18:32, Mathieu Lirzin a écrit : >> >> Michael Brohl writes: >> >>> This project is not only about tech, it has a user base with serious >>> business running on base of OFBiz. This has always to be considered as >>> serious as good technical solutions should be considered. So we cannot >>> simply change things because single contributors like other technical >>> solutions better. We have to remain downwards compatible and manage >>> deprecation of features carefully. >> >> First to clarify things, making evolutions in the framework is not about >> developers changing arbitrary stuff, it is about structuring internals >> in an understandeable way to enable correctness and the inclusion of new >> features that satisfies evolving requirements. > > Maybe you could clarify what you want to achieve. I have the feeling > that you have a long term view and the “component-load.xml change” is > only a step, right? Yes I am/was working on the support of Jar distribution of OFBiz allowing to separate the source code and compilation process of the framework from the development of plugins. Basically it means transforming OFBiz from a project template into a library to improve the extensibility and reusability of both the framework and the plugins. Moreover this work is contributing to the effort of facilitating the deployment of OFBiz in production environments (in the continuity of ‘gradlew distTar’) by allowing the move of specific configuration out of the source tree and deploying every resources inside the Jar. I have described the rationale for this work in [1]. > [...] >> For the record, Without the ability to safely refactor a large subset of >> the codebase that have the status of “implementation details”, I will >> simply stop contributing to OFBiz because I don't have time for endless >> discussions with people blaming my community work because their >> extensions happen to rely on unspecified behavior. > > For the current case, the most important question is to know if both > solutions could work at the same time, and if yes at which cost? Have > you an idea about that? Sure we can keep both options which is the easy way to settle a disagreement in the short term, I have allowed this in my last patch on OFBIZ-11296 [2] by supporting both and “component-load.xml” at the same time with a priority on “component-load.xml”. However to be able to continue the work of [1] it is important to remove the usage of “component-load.xml” inside the framework. [1] https://lists.apache.org/thread.html/c2612f1e296b6ea15872185871d3a9d83d6a4afc6d2a76f7a336a126%40%3Cdev.ofbiz.apache.org%3E [2] https://issues.apache.org/jira/secure/attachment/12989898/12989898_OFBIZ-11296_ignore-depends-on-when-a-component-load.xml-is-prese.patch -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Clarification about “removing component-load.xml feature” (was: What is OFBiz public API?)
Hello, Here are two clarifications about removing the “component-load.xml” feature. Michael Brohl writes: > Am 05.01.20 um 18:32 schrieb Mathieu Lirzin: > >> effectively optional (like the marketing example you brought) it should >> eventually be moved in the official plugins if we actually want to >> provides the capability for users to disable it. However users should > > Even it it would be a plugin, you still need a mechanism to > enable/disable it by configuration. > > To my understanding, if we use depends-on exclusively for framework, > applications and plugins, this would not be possible anymore. It depends if you consider modifying the “ofbiz-component.xml” as configuration or not. If yes there exists the “enabled” attribute of the element which provides a mechanism to disable a component. > Am 06.01.20 um 10:29 schrieb Samuel Trégouët: > >> 1. component-load.xml in plugin directory seems to be feature (nobody >> discuss this point) > > That *might* be a misunderstanding (if Mathieu agrees on this > point). My understanding was that he first implemented and committed > the change for framework and applications (on Nov. 25). > > From his mail in dev [1] and also the issue title of [2] I understand > that the component-load mechanism should be removed *everywhere* > afterwards. The dev mail would not make sense otherwise because > already committed the work at the time of writing (two weeks before) > and he announces to go on if noone objects. > > I apologize if I missed a point here, maybe Mathieu can clarify this? Even if I advocate for superseding “plugins/component-load.xml” with , I recognize it as a feature (documented briefly in the README). You are correct that in [1] I did not mention that feature and focus on the removal of “framework/base/component-load.xml” which is another potential user-space configuration mechanism. I could have spoken about the “plugins/component-load.xml” feature too but at that time I thought it was less likely to matter than “framework/base/component-load.xml” because of the immaturity of the plugins mechanism and the long term existence of “framework/base/component-load.xml”. Does it clarify things? >> [1]: >> https://lists.apache.org/thread.html/c2612f1e296b6ea15872185871d3a9d83d6a4afc6d2a76f7a336a126%40%3Cdev.ofbiz.apache.org%3E >> [2]: >> https://lists.apache.org/thread.html/7eab3d2ae3bbeadb184b02f75f7b2b86263532485e88ecba4d4dc780%40%3Cdev.ofbiz.apache.org%3E -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: What is OFBiz public API?
Hello, Jacques Le Roux writes: > Le 06/01/2020 à 10:29, Samuel Trégouët a écrit : >> >>what is OFBiz public API? >> >> In my opinion we need an answer for this question otherwise we need to >> discuss every single changement! which seems to be really cumbersome! >> And even if we discuss every single changement how to decide it is good >> for our community: *one* other contributor thumb-up is enough? maybe >> *two*? do we need to wait forever if others don't care about a >> particular changement? > > Mathieu and you make good points with the notion of "OFBiz public API". > It would be indeed good to have a such concept to officially (ie w/ a prior > consensus) collect all parts that always need deeper discussions and consents. > > But I fear it's not easy to define and this needs if not a deep discussion at > least a long one (see the kinda recursion here?). > > So before havi ng all agreed about what the "OFBiz public API" is I > think we need to cure the present issue. Except if Mathieu is pleased > to wait before this is agreed on. I think it is an important discussion that is overdue. Given that I need a break to both get over the frustration of the recent heated discussions and focus on my research work, As far as I am concerned this is a good moment for this discussion to happen. The goal of this discussion would be to define the boundaries between deprecated/stable/experimental/internal code by considering both the stability requirements that are important for production environments (that need to be able to upgrade smoothly) and the capability for developers to refactor code that is necessary to be able to implement the features allowing OFBiz to remain relevant in the future. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
What is OFBiz public API? (was: Removing “base/config/component-load.xml”)
Hello,
The arguments provided by Michael are very general and go beyond the
specific question of “component-load.xml” so I am opening a general
discussion about how to make OFBiz evolve smoothly by precising the
extent of its public API.
I urge other contributors to join this discussion which is crucial to
define our capability to work together as a community and my willing to
continue to participate.
Michael Brohl writes:
> This project is not only about tech, it has a user base with serious
> business running on base of OFBiz. This has always to be considered as
> serious as good technical solutions should be considered. So we cannot
> simply change things because single contributors like other technical
> solutions better. We have to remain downwards compatible and manage
> deprecation of features carefully.
First to clarify things, making evolutions in the framework is not about
developers changing arbitrary stuff, it is about structuring internals
in an understandeable way to enable correctness and the inclusion of new
features that satisfies evolving requirements.
Backwards compability only makes sense when something has a public API
otherwise every evolution is a breaking change. In OFBiz we lack a
proper specification of what is a feature provided by the framework
subject to backward compatibility and what is an implementation detail
that can evolve/disappear between version silently. We rely on an
informal consensus to distinguish between the two.
The fact that some mechanism appears to be used in production is a valid
argument against its removal only if that mechanism is part of the
public API, otherwise it is up to the client code to adapt.
My broad understanding of what is part of OFBiz public API is:
- the plugin mechanism
- the data model and data access (Entity Engine)
- The ability to call existing services and implement new ones (Service Engine)
- the HTTP routing mechanism (Event Handler)
- the various configuration files location in “{component}/config” directories.
> [...]
> If you read carefully what I previously wrote, there are several uses
> for the applications component-load.xml:
>
> * deactivation of unused component(s) by commenting out the
>load-component entry (why load marketing resources if you don't use
>the component at the moment)
> * addition of components (yes, I've seen projects where this was not
>done through the hot-deploy mechanism)
> * ordering these components in the right load order
>
> While you can argument that these might be "wrong" approaches, they
> are technically valid and used in customer projects. Therefore we
> cannot simply switch the mechanisms without a proper deprecation
> period.
The general problem here is not to know if things are wrong or
technically valid, it is to know if something is part of the public API
or is an implementation detail. This determines how to handle an
evolution on that part. Something wrong but part of the public API like
using XML for code should be handled with care (deprecation, migration
guides), but something technically valid but inappropriate like patching
framework Java source code from a plugin should be ignored.
In the case of ordering/enabling core components I consider it as an
implementation detail. If a component inside framework/applications is
effectively optional (like the marketing example you brought) it should
eventually be moved in the official plugins if we actually want to
provides the capability for users to disable it. However users should
not be entitled to think that they can freely desactivate/reorder/add
new components inside the framework/applications directory and that such
modifications will continue to work in a future release.
The larger question is about knowing if the internal organisation of the
files inside the "framework/applications" directories with the exception
of the “config” directories is considered part of OFBiz public API or
not? What do people think?
For the record, Without the ability to safely refactor a large subset of
the codebase that have the status of “implementation details”, I will
simply stop contributing to OFBiz because I don't have time for endless
discussions with people blaming my community work because their
extensions happen to rely on unspecified behavior.
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Happy new failing test suite year!
Hello Jacopo, Thanks for taking care of that. Jacopo Cappellato writes: > I have committed a fix for the trunk, all tests should pass now. Still > needs to be backported to the active release branches. > > Jacopo > > > On Fri, Jan 3, 2020 at 9:46 AM Jacques Le Roux > wrote: > >> Right, it's now failing on Buildbot >> https://ci.apache.org/projects/ofbiz/logs/trunk/framework/html/ >> >> The last one passed the 1st because, as mentioned Jacopo, it's almost the >> 2nd of January >> >> Jacques >> >> Le 03/01/2020 à 09:15, Jacopo Cappellato a écrit : >> > Hi Mathieu, >> > >> > it is actually probably related to the new Year... the demo data for the >> > financial period has a thruDate="2020-01-01 23:59:59.000" >> > I can have a look at this later today. >> > >> > Jacopo >> > >> > >> > On Thu, Jan 2, 2020 at 7:32 PM Mathieu Lirzin > > >> > wrote: >> > >> >> Hello, >> >> >> >> On my system I am noticing that the integration tests are failing with >> >> the following error snippet: >> >> >> >> --8<---cut here---start->8--- >> >> $ ./gradlew "ofbiz --test component=accounting --test >> >> suitename=accountingtests" >> >> [...] >> >> 2020-01-02 19:15:11,829 |main |TestRunContainer >> >>|I| --> >> >> >> auto-accounting-transaction-tests-purchase.testAcctgTransOnPaymentSentToSupplier(org.apache.ofbiz.testtools.SimpleMethodTest): >> >> Assertion failed: [acctgTrans.glJournalId=ERROR_JOURNAL] not-equals >> >> ERROR_JOURNAL as >> >> 2020-01-02 19:15:11,829 |main |TestRunContainer >> >>|I| junit.framework.AssertionFailedError: Assertion failed: >> >> [acctgTrans.glJournalId=ERROR_JOURNAL] not-equals ERROR_JOURNAL as >> >> at >> >> >> org.apache.ofbiz.testtools.SimpleMethodTest.run(SimpleMethodTest.java:98) >> >> at junit.framework.TestSuite.runTest(TestSuite.java:252) >> >> at junit.framework.TestSuite.run(TestSuite.java:247) >> >> at >> >> >> org.apache.ofbiz.testtools.TestRunContainer.start(TestRunContainer.java:90) >> >> at >> >> >> org.apache.ofbiz.base.container.ContainerLoader.startLoadedContainers(ContainerLoader.java:156) >> >> at >> >> >> org.apache.ofbiz.base.container.ContainerLoader.load(ContainerLoader.java:78) >> >> at >> >> >> org.apache.ofbiz.base.start.StartupControlPanel.loadContainers(StartupControlPanel.java:160) >> >> at >> >> >> org.apache.ofbiz.base.start.StartupControlPanel.start(StartupControlPanel.java:71) >> >> at org.apache.ofbiz.base.start.Start.main(Start.java:90) >> >> --8<---cut here---end--->8--- >> >> >> >> I was neither able to understand the issue nor bisect to find the origin >> >> of that failure. Since I remember running the integration tests less >> >> than a week ago, I am suspecting that this might be related to new >> >> year... I hope this is not actually the case. >> >> >> >> -- >> >> Mathieu Lirzin >> >> GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37 >> >> >> -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Adding a “--graph” option
Hello, In order to inspect what components are loaded by OFBiz and understand/debug their dependency relationship, it would be convenient to have a visual graph representation. I have opened OFBIZ-11315 [1] which adds a new “--graph” option which outputs a 'ofbiz.dot' file containing a textual representation of the components graph in the DOT format [2]. This file is meant to be processed by the Graphviz tool to get a png image: cat ofbiz.dot | dot -T png -o ofbiz.png Currently there is nothing very interesting to output in the graph because there is no “depends-on” relationship defined in the framework/applications components. To see some edges representing the dependency relation it is possible to re-apply commit eeabe69813a1d9f42911dec70a912574046ef49b Possible future steps could be to add some colors or to include other information like webapps, containers, ... in the graph. What do people think? [1] https://issues.apache.org/jira/browse/OFBIZ-11315 [2] https://en.wikipedia.org/wiki/DOT_%28graph_description_language%29 -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Happy new failing test suite year!
Hello, On my system I am noticing that the integration tests are failing with the following error snippet: --8<---cut here---start->8--- $ ./gradlew "ofbiz --test component=accounting --test suitename=accountingtests" [...] 2020-01-02 19:15:11,829 |main |TestRunContainer |I| --> auto-accounting-transaction-tests-purchase.testAcctgTransOnPaymentSentToSupplier(org.apache.ofbiz.testtools.SimpleMethodTest): Assertion failed: [acctgTrans.glJournalId=ERROR_JOURNAL] not-equals ERROR_JOURNAL as 2020-01-02 19:15:11,829 |main |TestRunContainer |I| junit.framework.AssertionFailedError: Assertion failed: [acctgTrans.glJournalId=ERROR_JOURNAL] not-equals ERROR_JOURNAL as at org.apache.ofbiz.testtools.SimpleMethodTest.run(SimpleMethodTest.java:98) at junit.framework.TestSuite.runTest(TestSuite.java:252) at junit.framework.TestSuite.run(TestSuite.java:247) at org.apache.ofbiz.testtools.TestRunContainer.start(TestRunContainer.java:90) at org.apache.ofbiz.base.container.ContainerLoader.startLoadedContainers(ContainerLoader.java:156) at org.apache.ofbiz.base.container.ContainerLoader.load(ContainerLoader.java:78) at org.apache.ofbiz.base.start.StartupControlPanel.loadContainers(StartupControlPanel.java:160) at org.apache.ofbiz.base.start.StartupControlPanel.start(StartupControlPanel.java:71) at org.apache.ofbiz.base.start.Start.main(Start.java:90) --8<---cut here---end--->8--- I was neither able to understand the issue nor bisect to find the origin of that failure. Since I remember running the integration tests less than a week ago, I am suspecting that this might be related to new year... I hope this is not actually the case. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: PortalPage / Portlet / parameters : SGBD versus XML file
Hello Olivier, Olivier Heintz writes: > In OFBiz, > * definition of portalPortlets, > * definition and content of the portalPages (column and which portlet in > each one) > * portalPortlet attributes for a portalPage > * and some other details about the organization of the portal > are stored in the ofbiz database. > > As Nicolas says, it is not practical/advisable to manage/sharing GUI > changes in a development process (and continuous integration) because > they are not managed by a VCS (svn or git). I share the feeling that it is not advisable to store complex behavioral elements like screen configurations inside the database because of the number of moving parts, which given the lack of safety nets when developping OFBiz screens is likely to end up in hours of debugging. > So I propose to > * create a widget-portal.xsd I have no opinion regarding the pertinence of portal screens in term of UI/UX so I let others judge on that, I am just a bit worry about the growth of the catalog of screen/form/... elements which for unfamiliar UI developper like me is already daunting. In any case providing a precise XML schema for screen/form elements is definitely a good idea. > * a new general property to read (or not) the database for the > organization of the portal > > * some modifications on include-portal to read the portal > information on the xml file and if the property is Yes the > complementary elements in the database. > > With these modifications it will be possible to dynamically add > portalPage or Portlet and use editParam features when working with > ProductOwner/keyUser/endUser but not in a production environment. > > After the workshop, the consultant will have to formalize what has > been validated in the appropriate xml file and commit it in order to > have it in production environment if the GUI test are ok (the > modifications don't break anything ;-) I think the scenario of letting “ProductOwner/keyUser/endUser” design a screen to convey their expectations to developpers would be a nice thing to have. As I understand it, such requirement does not imply the need for a general mechanism allowing the portal configuration to be retrieved from the database which would result in adding a lot of moving parts. One idea would be to have a specific webtools screen taking the form of a client-side Javascript program allowing users to compose a screen graphically and to export it as XML. This would fit the scenario you describe without the need of adding a general screen configuration mechanism inside the database. What do you think? -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Jacques Le Roux writes: > Mathieu asks Michael to provide an "explanation regarding why it > matters in production environments to be able to patch" > component-load.xml files @Michael: ping -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Reverting commits guidelines
Hello Taher, Taher Alkhateeb writes: > Also, if there are disagreements on a direction among members then it is > usually the case that a -1 from a binding vote requires a revert and > starting a discussion. Then we can either have consensus, lazy consensus or > the issue is shut down. > > I'm saying what should be done, just laying out the standard rules we > usually run by. For more precision, a veto for code-modification is not enough for justifying a revert [1]: --8<---cut here---start->8--- To prevent vetos from being used capriciously, they must be accompanied by a technical justification showing why the change is bad (opens a security exposure, negativelyaffects performance, etc. ). A veto without a justification is invalid and has no weight. --8<---cut here---end--->8--- [1] https://www.apache.org/foundation/voting.html#Veto -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Hello Michael, Michael Brohl writes: > inline... > > Am 18.12.19 um 00:01 schrieb Mathieu Lirzin: >> Hello, >> >> @Michael: I would like to know if you intend to provide an explanation >> regarding why it matters in production environments to be able to patch >> “framework/component-load.xml” and “applications/component-load.xml” ? >> This would help determining what should be done regarding [1] (meaning >> documentation, reverting, migration guide, ...). Sorry if I am pressing >> you but I need that the discussion moves forward. > > > Why do you "need that discussion moves forward"? I do not see any > reason to hurry. This need is practical because when I speak about *things* that seems blocking for people, I can't just continue to implement the next step of those *things* in parallel by ignoring people input, so I am effectively blocked by this discussion. > In my view, you still did not explain sufficiently why we need this change. > > I summarize: > > * you introduced a *new* mechanism with the goal to remove a field > tested one > * it does not provide any new functionality, better readability or > fixes a bug > > * it forces users to fix their existing installations once they use a > new release with this change I have told you multiple times that this was not my interpretation of what happened. I would have appreciated if your interpretation could be expressed in a modularized way meaning that this is your interpretation not a proven fact. > * you did not answer my questions if this is tested working togehther > with component-load.xml To be honest I intentionnally ignored this question to keep the discussion positive and constructive, because in the same paragraph you were asking for unreasonable things by stating “It is not my responsibility to prove that it is not working, it is yours to prove that it works.” which as every serious software developper know is impossible unless a formal proof [1] is provided which is unrealistic in OFBiz context. I did not explicitly test the scenario you described because I am still unable to see even after asking you multiple times for an explanation and more context, why this scenario matters. Because Samuel is a nice and constructive person, he has tried the described scenario, and discovered that there is actually a regression in [3] meaning that “component-load.xml” does not fully override the declarations. So for your pleasure the commit is going to be reverted. \o/ > PS: I'd suggest to open a feature branch with the changes and upcoming > changes. This removes pressure to get things into the trunk just to > move forward and would allow the community to study the approach > without the downside to break anything in trunk before the solution is > worked out as a whole. That is an option, however creating a feature branch has trade-offs, meaning the longer it stays alive the more difficult it becomes mergeable. My impression is that a feature branch for the “Location independent extension mechanism” work [2] will necessarly be long-lived with a lot of impact in the framework. As a consequence that branch will quickly be unmergeable and become effectively a fork. As a consequence I would rather prefer to keep the active development of [2] in trunk to avoid creating a fork. My understanding is that the big changes that Taher introduced some years ago (Plugins, Gradle, Startup) were done with overall community consensus on trunk, but maybe I am overlooking some important details. [1] https://en.wikipedia.org/wiki/Computer-assisted_proof [2] https://lists.apache.org/thread.html/c2612f1e296b6ea15872185871d3a9d83d6a4afc6d2a76f7a336a126%40%3Cdev.ofbiz.apache.org%3E [3] https://github.com/apache/ofbiz-framework/commit/eeabe69813a1d9f42911dec70a912574046ef49b -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Hello, @Michael: I would like to know if you intend to provide an explanation regarding why it matters in production environments to be able to patch “framework/component-load.xml” and “applications/component-load.xml” ? This would help determining what should be done regarding [1] (meaning documentation, reverting, migration guide, ...). Sorry if I am pressing you but I need that the discussion moves forward. If not I would like to refocus the discussion on the next step which is about the propositions of: 1. Removing “base/config/component-load.xml” and hard-code the ["framework", "themes", "applications", "plugins"] sequence with the new requirement that extensions must be done inside “plugins” as long as we don't have a location-independent extension mechanism [2]. 2. Removing the “component-load.xml” feature which provides a mechanism to define an arbitrary order for a set of components inside a directory to bypass the “smart loading” derived from functional component dependency declarations. This removal is a pre-requisite to implement a location-independent extension mechanism [2]. Are there any objection regarding those 2 propositions ? [1] https://github.com/apache/ofbiz-framework/commit/eeabe69813a1d9f42911dec70a912574046ef49b [2] https://lists.apache.org/thread.html/c2612f1e296b6ea15872185871d3a9d83d6a4afc6d2a76f7a336a126%40%3Cdev.ofbiz.apache.org%3E Mathieu Lirzin writes: > Michael Brohl writes: > >> I am still not sure why we need the new mechanism. >> >> And if we decide to use both, we should make sure that the old version >> is the default at least for the lifecycle of one release with proper >> an clear dopcumentation for users. >> >> Thanks, >> >> Michael >> >> >> PS: I'm asking myself why some people have such a big problem >> reverting their work if others object against it. If there was no >> review/discussion/consensus for a new feature, it should simply not go >> into the codebase and should at least be reverted if there are >> objections. >> >> It's tiring to discuss this afterwards and if the people objecting are >> not persistent enough, the code simply stays. > > I have personally no problem reverting things. If you look at the ‘git > log’ you will see some recent reverts I have made. I just need to > understand the actual objection before reverting [1]. > > Since your argument seems to be about the “impacts on users” an > explanation regarding what you or others are actually achieving when > patching the “framework/component-load.xml” and > “applications/component-load.xml” would help me understand the issue, > because I honestly do not see why the loading order/mechanism of > “framework” or “applications” should not be considered an implementation > detail. > > By the way to give more context on my perspective, the usage of > instead of “component-load.xml” in the > framework/applications directories is related to the implementation of > the work described in a previous discussion [2] because it defines a > location independent an extensible component loading order. > > HTH, > > [1] > https://github.com/apache/ofbiz-framework/commit/eeabe69813a1d9f42911dec70a912574046ef49b > [2] > https://lists.apache.org/thread.html/c2612f1e296b6ea15872185871d3a9d83d6a4afc6d2a76f7a336a126%40%3Cdev.ofbiz.apache.org%3E -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Hello Michael, Michael Brohl writes: > I am still not sure why we need the new mechanism. > > And if we decide to use both, we should make sure that the old version > is the default at least for the lifecycle of one release with proper > an clear dopcumentation for users. > > Thanks, > > Michael > > > PS: I'm asking myself why some people have such a big problem > reverting their work if others object against it. If there was no > review/discussion/consensus for a new feature, it should simply not go > into the codebase and should at least be reverted if there are > objections. > > It's tiring to discuss this afterwards and if the people objecting are > not persistent enough, the code simply stays. I have personally no problem reverting things. If you look at the ‘git log’ you will see some recent reverts I have made. I just need to understand the actual objection before reverting [1]. Since your argument seems to be about the “impacts on users” an explanation regarding what you or others are actually achieving when patching the “framework/component-load.xml” and “applications/component-load.xml” would help me understand the issue, because I honestly do not see why the loading order/mechanism of “framework” or “applications” should not be considered an implementation detail. By the way to give more context on my perspective, the usage of instead of “component-load.xml” in the framework/applications directories is related to the implementation of the work described in a previous discussion [2] because it defines a location independent an extensible component loading order. HTH, [1] https://github.com/apache/ofbiz-framework/commit/eeabe69813a1d9f42911dec70a912574046ef49b [2] https://lists.apache.org/thread.html/c2612f1e296b6ea15872185871d3a9d83d6a4afc6d2a76f7a336a126%40%3Cdev.ofbiz.apache.org%3E -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Hello Taher, Taher Alkhateeb writes: > I was involved in some of the biggest changes in the framework > (gradle, unit tests, start component, core framework, etc ...) and > every time it involved a good deep discussion on the mailing list > trying to reach consensus before implementation. > > So I recommend always treading carefully when doing larger changes and > getting others involved. Sometimes people surprise you with better or > simpler solutions if you give them the opportunity to express their > opinion. This is my perspective too, and every time I feel that an improvement I am considering has a strong impact, I make the effort of sending an email to this ML explaining the problem and asking for feedback on the considered solution. > Now regarding completely replacing the component-load.xml with > depends-on, I'm not sure this makes much sense at the core framework > level for a few reasons: > > - First I don't think this issue is linear. It is possible for > example, to speed up the startup time of the system by running things > in parallel where possible > - Also, the multi-threaded portion of the system (some of it using the > executor framework in java.util.concurrent) needs to be studied > carefully to see how the loading sequence is best optimized > - Finally, I'm just not convinced of either approach (depends-on or > container-load) for core framework functionality. We need a better > solution that handles the core quite differently (without the need for > a component concept). > > So a better refactoring IMHO on the core level involves an > architecture that does not necessarily comply with the component > concept, and we can proceed with the original plan of further breaking > down the system into a core deplorable framework, core components, and > plugins. Since you don't like either solutions and given that currently we have both, wouldn't it be a step forward to get rid of one of them ? :-) IMO the discussion is not about deciding if separating the framework core into multiple components is a good idea or not. We are only discussing if we should remove the “component-load.xml” feature which implies removing an historic configuration mechanism that was providing integrators the capability to use other meta-directories than "framework", "themes", "applications, "plugins" and to define a total order between components in component directories. Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Michael Brohl writes: > I was in fact asking for the discussion and review process to the > changes already introduced and committed in the mentioned Jira. OK > It's a good approach to let fellow committers review such work before > it gets committed to the codebase. I agree in theory, but given the poor state of the codebase I feel that requiring that every single change is delayed for 2/3 days or more to let people review it is not manageable. The approach I am following in practice is to apply my own judgement to separate things that are safe and things that need to be discussed beforehand. I considered (maybe wrongly) that using in the “framework” and “applications” components directories was safe because it did not remove any feature. > This change will affect existing productive installations and should > not be made without proper documentation and clear instructions for > the user on how to migrate their installation to the new mechanism. > > To me it is not clear what we are gaining with this change. We are > removing one tag which is used and stable for years and introduce > another one which was not used before. Users are forced to migrate > their installations if they had custom modifications. I am confused about what you are talking about because I still don't see in what has already been committed in ‘trunk’ what is “affecting productive installations” because the “component-load.xml” feature is still here and users can still place their existing “component-load.xml” files and get the same behavior as before. > I did not look deeply into the changes yet but it looks strange that a > component like "product" or "order" only depends on "humanres". In > fact, they have more dependencies like "party". Given that > "depends-on" really means what it says and has a real difference to > the component-load.xml approach. The dependency relation is transitive meaning: if (x depends on y) and (y depends on z) then (x depends on z). It is possible to be explicit about the "transitive" dependencies by declaring them as direct dependencies when the fact that a direct dependency is depending on another one is an implementation details which might not hold in the future. For example: the Gradle plugin “groovy” depends on the Gradle plugin “java” but there is little chance that the “groovy” plugin will not depend on the “java” one in the future so it is safe to only declare a dependency on the “groovy” plugin. The current dependency declarations in framework/applications is a direct mapping of the order defined in the previous “component-load.xml” file which need to be refined because that order lacked information regarding the actual functional dependencies (which is the inherent downside of using a total order for partially ordered things). > The component-load.xml mechanism clearly shows the loading order of > the components which is an advantage over the cluttered information of > the depends-on mechanism. You will have to analyze every > ofbiz-component to see what's going on. You are correct that “component-load.xml” shows the precise loading order of components, however it lacks a precise definition of the actual functional dependencies between components. You can derive one valid loading order from the set of actual functional dependencies, however as stated above it is not possible to derive the functional dependencies from a “component-load.xml” file. Currently we lack a mechanism to display the global dependency graph which is important to for example to analyse why a component is loaded before or after another one. However this feature should be easy to implement and could be a requirement before removing the “component-load.xml” feature. > IMO these examples show that is in fact worth a discussion and should > not be a lone decision of a single committer. > > I would like to see this being reverted and proposed for discussion > and review before this is going to be introduced into the codebase. I have no problem reverting things, however to be legitim your call for revert should be backed by an actual proof that the “component-load.xml” feature has been removed (which is not the case) or that the component loading work has introduced a bug/regression. So I encourage you to try to backport one of your production “component-load.xml” on trunk and report a bug if necessary. Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: [ofbiz-framework] branch trunk updated: Fixed: Fix missing else during previous refactoring (OFBIZ-11253) When you rendering for with auto-field-entity on edit mode, indicator field has been overr
Nicolas Malin writes: > Effectively, good point to make more attention on the future. > > I'm not sure that I can amend this commit without make a push force > after, so I prefer to keep this error instead generate more > inconvenient. Indeed that was my suggestion to follow this convention in your future commits, not to rewrite the history (which hopefully is not allowed by Gitbox on ‘trunk’). > thanks also Jacques for the following issue :) Thanks! -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Removing “base/config/component-load.xml”
Hello Michael, Michael Brohl writes: > can you please point me to the discussion where this important change > was discussed before it was introduced? > > I only find the Jira https://issues.apache.org/jira/browse/OFBIZ-11296 > which was closed only hours after it was created. If you speak about the usage of in the “framework” and “applications” directory, See my response in OFBIZ-11296 regarding what has actually changed. Regarding what I am proposing in this email, I did not open a Jira yet and did not commit anything removing the “component-load.xml” feature. Sorry if I was not explicit enough about the fact that this is a discussion not a report of things already changed. > Am 08.12.19 um 02:34 schrieb Mathieu Lirzin: > [...] > >> In order to simplify things which helps the endeavour towards >> transforming OFBiz in an extensible JVM based library, I would like to >> remove such configuration point and hard-code the list of component >> directories inside the code. >> >> [...] >> >> If nobody step-up in a week, I will go ahead. >> >> Thanks. >> > -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Removing “base/config/component-load.xml”
Hello, We have in OFBiz two redundant ways to define the order in which components are loaded. * “component-load.xml” files stored in components directories meaning directories containing single components. They define a total order between every component inside that directory. * XML elements inside “ofbiz-component.xml” files stating that a component ‘c’ requires a set of components ‘D’ to exist and to be loaded before trying to load ‘c’. Globally this constructs a dependency graph which defines a partial order between available components. Currently is used everywhere in the framework on ‘trunk’ because it is more declarative and open for extensibility due the partial ordering capability. The only exception is “base/config/component-load.xml” which is a special components directory configuration defining the order of other components directories meaning “framework”, “themes”, “applications” and “plugins” in their respective dependency order. This file allows integrators to add and reorder components directories. This feature is not really useful nowadays considering that our plugin mechanism provides sufficient extensibility capability when combined with definitions. In order to simplify things which helps the endeavour towards transforming OFBiz in an extensible JVM based library, I would like to remove such configuration point and hard-code the list of component directories inside the code. I guess this low-level technical proposition is not interesting for most people but in case someone want to understand more or object before things are actually removed. I am opening a discussion on this ML. If nobody step-up in a week, I will go ahead. Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: [ofbiz-framework] branch trunk updated: Fixed: Fix missing else during previous refactoring (OFBIZ-11253) When you rendering for with auto-field-entity on edit mode, indicator field has been overr
Hello Nico, nma...@apache.org writes: > commit 071a74238b4d53fb95ffd214e0e68e55840a28e3 > Author: Nicolas Malin > AuthorDate: Fri Dec 6 21:29:01 2019 +0100 > > Fixed: Fix missing else during previous refactoring > (OFBIZ-11253) > When you rendering for with auto-field-entity on edit mode, indicator > field has been override by text field. > Fix also two error on source field type set to > FieldInfo.SOURCE_AUTO_SERVICE instead of FieldInfo.SOURCE_AUTO_ENTITY > > Thanks to Olivier Heintz to inform on the regression. > --- In your following commits, it would be nice if you could add an empty line between the summary and body of your commit messages. This will help ‘git log --oneline’ to be more helpful by not showing the details of the commit. Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: OFBizDebug and OFBIZ-11302
Jacques Le Roux writes: > Le 02/12/2019 à 18:18, Mathieu Lirzin a écrit : >> >> Jacques Le Roux writes: >> >>> With OFBIZ-11302 Mathieu removed the ofbizDebug Gradle task, because "ofbiz >>> --debug-jvm" can be used instead. >> This is now ‘gradlew "ofbiz" --debug-jvm’ with the --debug-jvm outside >> of the double quotes. > > g ofbiz --debug-jvm (no quotes at all) works on Windows (I use g as shortcut > to gradlew) Yes that works with Bash too. But if you want to pass options to ofbiz you need to add the quotes in order for the gradle template tasks to match, like ‘gradlew "ofbiz --start" --debug-jvm’. IMHO the fact that someone like you which is a seasoned OFBiz contributor is getting confused by the syntax of the syntactic sugar provided by task templates is a strong sign we should *not* use this kind of obscure black magic and use the standard and *documented* ‘run’ task instead [1] which would read like the following: gradlew run --args="--start" --debug-jvm [1] https://docs.gradle.org/current/userguide/application_plugin.html >> >>> I have no problem with this change, it's easy to remember. Though I >>> like the idea of having syntax sugar to easy remember commands, like >>> ofbizDebug and ofbizBackground >>> >>> BTW ofbizDebug still appears when you run "gradlew tasks". And don't we >>> lose the "" part? >> On my machine ‘gradlew tasks’ do not include ofbizDebug on trunk. > > Right, I must have tested before pulling or something > > So, if nobody is against this change, I'll create a new shortcut for the > whole command, et voilà :) Not a problem with me. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Github PRs and Jira
Hello, As far as I am concerned, I will never be enthousiastic regarding adopting yet another proprietary platform. Gil Portenseigne writes: > +1 to use gitlab features to work around code, that will help > collaboration. > > I don't know if there is an Apache alternative to get the features without > Microsoft. But having two official tools to get contribution is not desired. I agree that we should not have multiple workflow because contributing process is already complex enough to not add yet another option. I think that I we adopt Github workflow, we should abandon Jira. In term of ethics, we would just replace an evil proprietary software with another one, so I won't oppose. :-) > I guess that using existent Github should be ok for official pull > request, if not, it's always ok to have and attached patch or > other gitlab reference. You mean "Github reference" I guess ? > > Le 11:55 - mardi 03 déc., Jacques Le Roux a écrit : >> Le 03/12/2019 à 08:21, Jacopo Cappellato a écrit : >> > Now that we have migrated to Git, in my opinion we should really consider >> > to adopt the workflow based on PR as our primary method for accepting >> > contributions. I understand that it is a relevant change for the community, >> > but I also see many advantages and a more natural (for Git) workflow for >> > contributors and committers. Contributors would fork from the official repo >> > and would submit pull requests that the committers would merge into the >> > official branches. >> > >> I tend to agree, it's simple to merge and commit. We will still maybe need >> Jiras when discussions are needed, or in all cases to fill the blog monthly >> posts? >> >> I guess using Github (ie Microsoft :D) is not a problem for the community? >> >> Opinions are welcome >> >> Jacques >> -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: OFBizDebug and OFBIZ-11302
Hello, Jacques Le Roux writes: > With OFBIZ-11302 Mathieu removed the ofbizDebug Gradle task, because "ofbiz > --debug-jvm" can be used instead. This is now ‘gradlew "ofbiz" --debug-jvm’ with the --debug-jvm outside of the double quotes. > I have no problem with this change, it's easy to remember. Though I > like the idea of having syntax sugar to easy remember commands, like > ofbizDebug and ofbizBackground > > BTW ofbizDebug still appears when you run "gradlew tasks". And don't we lose > the "" part? On my machine ‘gradlew tasks’ do not include ofbizDebug on trunk. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: [POC Vue.Js] first presentation / summary
Olivier Heintz writes: >> Would you be interested in spending some time integrating this view handler >> inside the framework? > > of course, I can created a Jira with the java and update them when > there are remarks or discussions Excellent. :-) >> The requirements for such task would be to lint the code, write some >> integration tests, some javadoc, some comments explaining the non-obvious >> parts of the implementation, ... well a non-negligeable amount of work which >> is important for maintainability reasons. > the amount of work is not the main problem ;-) > My knowledge and skill will be more difficult to solve, > I am a business/functional consultant and so a java copy/paste developer (or > a "java simple/procedural" business process developer) > not a java framework developer. > In the team I'm working, other Co-workers are front-end developer or profile > similar than me. > > So, > 1) javadoc no problem and documentation in adoc format no problem too For a JSON screen view handler I guess most documentation effort would be in Javadoc making clear what kind of XML elements is handled by a method, what cases are considered and what are the expected effects, stating if ‘null’ values are accepted and potentially returned. The issue is that some interfaces like ‘FormStringRenderer’ are not documented so it would be nice to add the documentation for those interface too. The following link provides good advice and conventions for Javadoc: https://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#terminology > 2) cleaning code, and applying best practice, no problem if reviewer can > give me some link or example Works for me. The first step is to remove commented code and run ‘gradlew check’, to ensure that before and after your changes no linting errors are introduced (alternatively you can configure your IDE to run ‘checkstyle’ lint tool on your source files). I can help with best practice guidance if needed. > 3) integration test, when I read the ModelFormFieldTest.java I don't > understand what it does and the java ... so I will need some help for this > sub-task ModelFormFieldTest is indeed tricky to understand because the semantics of the tested feature is complex (which motivated writing those tests in the first place). I guess what is hard for you to understand is related to the usage of lambdas denoted by ‘… -> …’ and passed as parameter to the ‘ModelFormField#from(Consumer)’ static method ? IMO There is no need to use lambdas to write maintainable code so this is not a blocker. Regarding the kind of tests that matters for JSON rendering, it is important for example to make sure that screen values are mapped to an adequate JSON types (for example that boolean, numbers, list of values), that elements are run and have the expected effects, that pagination is working and that ‘null’ values are handled as intended. >> Moreover on the features side, it might be interesting to make the >> presentation related stuff optional for consumers only interested by the data >> aspect of the data. >> >> What do you think? > Yes it's in the POC plan, but currently we are more focus on having a clear > user demonstration in a business environment. > This month a large part of Party component have been done and we are working > on solving all bugs/"form xml functionality not yet implemented" > Hoping to have a usable demo next week. > (https://ofbiz-selenium.ofbizextra.org/partymgrfjs/control/showPortalPage?portalPageId=PartyMgmtFrontJs) > > Next week I will do a mail with the detail of what is usable and which next > step are possible > If you or community think that implemented this option is a priority, we will > do it, I think it will not too difficult. Since you seem OK with spending some time in the reviewing process, feel free to open a ticket on Jira with a first version that I can start reviewing. Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: buildbot exception in on ofbizTrunkFrameworkPlugins
Hello Jacques, Jacques Le Roux writes: > OK, now we need to agree about tasks.checkstyleMain.maxErrors in build.gradle: > > https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/1041/steps/shell_1/logs/stdio > > Locally with trunk and plugins HEAD I get > Checkstyle files with violations: 1039 > Checkstyle violations by severity: [error:37854] Probably because you have an extra plugin installed and/or an different version of the official plugins. > I still wonder why it's not, as in Buildbot > Checkstyle files with violations: 1039 > Checkstyle violations by severity: [error:37787] With latest trunk from both framework/plugins repository I get the same number on my machine. > And why we have tasks.checkstyleMain.maxErrors = 37776 in code. 37787 - 37776 = 11 linting errors This is the number of linting errors introduced by either Pawan Verma or Nicolas Malin since commit a521de9ad8a0b5a0f9ceadab348c46d7961ff89a that remains not fixed. > Could someone enlighten me? Does it help ? :-) -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: buildbot failure in on ofbizBranch17FrameworkPlugins
Jacques Le Roux writes: > Also we get it in the Blamelist (also in Git), but Samuel is not a committer > :/ See my response to your other email for a rationale explaining why we got to Samuel appearing in the blame list. > Le 27/11/2019 à 19:28, build...@apache.org a écrit : >> The Buildbot has detected a new failure on builder >> ofbizBranch17FrameworkPlugins while building ofbizTrunkFramework. Full >> details are available at: >> https://ci.apache.org/builders/ofbizBranch17FrameworkPlugins/builds/418 >> >> Buildbot URL: https://ci.apache.org/ >> >> Buildslave for this Build: asf946_ubuntu >> >> Build Reason: downstream >> Build Source Stamp: [branch release17.12] >> d1c037dca1ea14caf545c85c3741bb9af093f3c9 >> Blamelist: Samuel Trégouët >> >> BUILD FAILED: failed shell_5 >> >> Sincerely, >> -The Buildbot >> >> >> >> > -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: [ofbiz-framework] branch release17.12 updated: Fixed: add XML declaration in “web.xml” files (OFBIZ-6993)
Hello Jacques, Jacques Le Roux writes: > Not a big deal (it's only in the commits log), I'm not sure we want stuff > like: > > Le 27/11/2019 à 17:48, m...@apache.org a écrit : >> Author: Samuel Trégouët > > Normally we rather use something like > >Thanks to Samuel Tregouet for this fix > > I did not notice yet, we have already 10 of them in Git repo. > > Not sure we want to avoid them, maybe if Samuel has an ICLA it's OK, unsure > on my side... > > I mean is that something Git forces us, or could we prevent it if needed? The fact that Samuel was defined as the author of the commit was a conscient action on my side (i.e. Git do not force us to do so). Since it is a very common practice to commit somebody else work, in Git there is a clear distinction between the author and the committer which can is done with ‘git commit --author="..."’ when both are not the same person. This provides more explicit credits to the actual author without relying on ad-hoc conventions. As I understand it, the practice of committing somebody else work under the committer name (not the author name) and adding credits in the commit log to the actual author was only the result of a limitation of Subversion which does not handle the scenario of committing somebody else work. I didn't bother discuss the usage of this Git feature before hand because I thought it would be obviously accepted by the community, but since it seems not necessarily natural for you (an probably others) I am open to the discussion now. :-) Indeed I am aware that any author should sign an ICLA, I assumed that Samuel have signed but did not actually check. Samuel: have you signed the ICLA ? -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: [POC Vue.Js] first presentation / summary
Hello Olivier, Olivier Heintz writes: > As explained in a previous mail, we (me and others co-worker in a company I > am working for) are working on a POC for using Vue.js as GUI generated > from the current screens/forms/menu xml ofbiz files. > The customer goals is to be able to migrate a lot of existing Portal / > portlet developed with ofbiz R13.07 on trunk and on a modern GUI. > > The main customer specification / requirement are, for the first step of > migration > 1. minimum of modifications on xml file, to have the easier and quicker > migration > 2. same GUI rules, to facilitate user acceptance > 3. new GUI for a component should be working with classic GUI for other > components > Enhancement can be possible but will be applied in a second step with > productOwner check and validation. > > Current POC status is : done with example for some screen / field /menu, > waiting user testing feed back before starting partymgr migration. > > Our demo platform is > https://ofbiz-selenium.ofbizextra.org/examplefjs/control/main with admin / > ofbiz > > the chosen architecture is, > 1. a new ScreenViewHandler which > - use new renderer for screen / form /menu > - to return a json with 2 map > * ViewScreen with detail about presentation (similar to xml tag and properies) > * ViewEntity with data to populate ui generated (extract from screen/forms > data) Providing a JSON view handler is very interesting because it enables any kind of Javascript front-end to consume data rendered by existing OFBiz screens (with data preparation, etc). This contributes to the current effort of implementing a REST API by enabling consumers to have a JSON representation of the retrieved data which is convenient to manipulate from Javascript [1]. Would you be interested in spending some time integrating this view handler inside the framework? The requirements for such task would be to lint the code, write some integration tests, some javadoc, some comments explaining the non-obvious parts of the implementation, ... well a non-negligeable amount of work which is important for maintainability reasons. For example one requirement would be to use ‘org.apache.ofbiz.base.lang.JSON’ instead of ‘org.json.simple.JSONObject’ in order to avoid depending on multiple JSON libraries. Moreover on the features side, it might be interesting to make the presentation related stuff optional for consumers only interested by the data aspect of the data. What do you think? [1] https://issues.apache.org/jira/browse/OFBIZ-4274 -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: linting issues on ‘trunk’, ‘gradlew check’ fails.
Jacques Le Roux writes: > Since it depends on the good will of scrutinizer/s and varies in > errors depending on context, I suggest that we add a "gradlew check" > step in our Builbot trunk framework builder. > > Then everyone will be aware of this feature and we will be able to adjust the > maxErrors as needed. Actually I'm still waiting for INFRA-19443 for that... Excellent idea. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
linting issues on ‘trunk’, ‘gradlew check’ fails.
I recently pulled the latest commit, and I noticed that latest commits have introducted linting issues. Since a521de9ad8a0b5a0f9ceadab348c46d7961ff89a ‘gradlew check’ fails. Pawan Verma, Nicolas Malin and Jacques Le Roux can you fix the java code you have written to conform to OFBiz coding style ? Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Change commit message template?
Hello, Jacques Le Roux writes: > Le 17/11/2019 à 19:16, Taher Alkhateeb a écrit : > >> I'm not sure what convention over configuration has to do with what >> you're saying :) > > It was because of this sentence in the reference link > >"This is a project specific convention that has no relation to any > requirements or limitations in Git. Yes, it is a fine convention for many > projects but it should not be required." > > And yes I abused the concept a bit :) I see our template as a configuration. > >> Anyway, it makes sense. Most GIT editors consider the first line to be >> subject, and third line onward is the body of the message. So perhaps >> we can move the (OFBIZ-) to the end of the first line or maybe >> make it the last line in the body or something like that. > > Finally, like Gil, I prefer to keep the 2 infos on the same line. It > would make less sense to have the issue number inside the body of the > message IMO to guess what make sense or not, I think considering the usage of that information matters. For instance a lot of UI ontop of Git show only the summary (the first line) in a first step to give an overview of the commit and require a user action to display the details (the body) of the message commit. So if we choose to put the ticket number in the summary it is because we find valuable to see it at first glance otherwise it is just noise. In other free software communities I work with, the bug ticket number was only considered as metadata (like our “thanks” part) put in the body so that it can be easily retrieved with ‘git log --grep=...’ when diving in the history. I personally don't see any value in having auto generated identifiers inside the summary of our commits, but if people in this community find it useful in their workflow, I don't mind following that convention. -- Mathieu Lirzin
Re: Git repo for each ofbiz plugin
Hello Jacques, Taher and Michael, I am realizing that I have overreacted to Jacques question by turning it into a vague (not that constructive) debate about extension mechanism for providing plugins. I think the question of removing push/pull/removePlugin* commands should be post-poned when (1) a viable alternative is ready to use and adapted to non-technical savy users, or (2) nobody want to devote time to fixing issues related to those commands. Since neither (1) or (2) is currently the case I retract my suggestion and +1 for Jacques solution. Sorry for letting my frustration regarding OFBiz extensibility influenced my initial answer. Kind regards, Taher Alkhateeb writes: > If you study the plugin system implementation, you will notice that it > is based on maven for dependency resolution, which already takes care > of many of the details you mentioned, especially with respect to > dependency management. The way it is designed avoids reinventing the > wheel by using something that already solves the problem. Now granted > it is incomplete, that doesn't mean it's a bad idea, It's just > incomplete. And part of the complexity by the way, is deficiencies in > the architecture of OFBiz itself. The concept of a "component" needs > to improve and better utilization of the subproject architecture would > greatly improve this. > > I don't have a problem with removing whatever you feel like removing. > I only propose a more thorough assessment before shrugging it off as > "too complex" or "poor". Dependency management by design is complex. > And using Jar libraries or any other solution will not make it easier, > quite the opposite. It does not matter whether it lives in the build > system or somewhere else (although I personally prefer the build > system). The important thing is how to architect it to solve your > problems. > > Anyway, I have no further input on this, please feel free to discuss > this with the rest of the folks and reach whatever decision you feel > like heading to. > > On Tue, Nov 12, 2019 at 1:26 PM Mathieu Lirzin > wrote: >> >> Taher Alkhateeb writes: >> >> > I'm not sure I see this issue with the same perspective. Having the >> > build system automatically download (and run some code) for a certain >> > component is a convenience especially for the community components. >> > Things are even easier when using git as there are many plugins to >> > support that. I'm also not sure what is "poor" about that >> >> I agree that having the “build system” downloading things automatically >> is convenient and desirable. But IMO what we should mean by a build >> system is Gradle (or Maven, SBT, Leinigen, ...) not “OFBiz pretending to >> be a build system ontop of Gradle”. >> >> Let me clarify what I mean by “OFBiz build system doing things poorly”. >> There are a lot a important features that people would expect from a >> tool downloading/installing plugins (a.k.a package manager) that are >> missing like for example: >> >> - having a manifest listing the installed plugins with their version >> - removing unneeded dependencies when uninstalling a plugin, >> - having a way to upgrade an install plugin (automatically upgrading its >> dependencies if needed). >> - Ensuring that the plugin is compatible with the framework version >> - Resolve plugin version mismatchs consistently (A depends on B-0.1 and C >> depends on B-0.2 => use B-0.2) >> - Having a shared cache for already downloaded dependencies. >> >> One could then say “we just need to implement those features” to do >> things properly. But having some experience with in the domain of >> package managers, I know that things are too *complex* for us OFBiz >> developpers to mess with that. This is why I am proposing to remove the >> push/pull/remove commands and advocate towards working on the adoption >> of existing distribution interfaces like Jar libraries (See work done in >> OFBIZ-11161 [1]). >> >> [1] https://issues.apache.org/jira/browse/OFBIZ-11161 >> >> > On Mon, Nov 11, 2019 at 10:37 PM Mathieu Lirzin >> > wrote: >> >> >> >> Hello, >> >> >> >> Jacques Le Roux writes: >> >> >> >> > I propose that waiting for plugins Maven repos we simply continue to >> >> > use Svn through Github as I explained elsewhere >> >> >> >> No need to wait for Maven plugins before acting responsibly. :-) >> >> >> >> A preferable solution would be to simply remove the >> >> ‘push/pull/removePlugin*
Re: Git repo for each ofbiz plugin
Hello Michael, Michael Brohl writes: > why using strong words and disrespect other community people's work > like this? This does not help in any way making things better. If I showed disrespect to people in that community I am sorry about that, that was not intended. I was just trying to tell that we might be better off providing those package management commands that are lacking important features, are troublesome to maintain. > Having said this, to the topic: users often have difficulties even to > get OFBiz running while it seams easiest for us developers. Just read > the users list for reference. > > Having everything available and be done automatically helps people to > get things running easier. By removing these features we would make > things more complicated for them. This is true when things works reliably. But when things fails (which is expected with tinkered tools) it is worse for users who are then helpless because of the added complexity, when instead we could have just expect them to know how to download a repository and place it in particular directory which is not a hard requirement IMO. > More advanced people who are familiar with the mentioned techniques > can use them anyway. In the case of plugin management, I claim that we can make things easier for everyone (developpers, users) by adopting well known Jar distribution mechanism to manage both the framework and the plugins installation (but that is ongoing work [1]). [1] https://issues.apache.org/jira/browse/OFBIZ-11161 > > Regards, > > Michael > > Am 11.11.19 um 20:37 schrieb Mathieu Lirzin: >> Hello, >> >> Jacques Le Roux writes: >> >>> I propose that waiting for plugins Maven repos we simply continue to >>> use Svn through Github as I explained elsewhere >> No need to wait for Maven plugins before acting responsibly. :-) >> >> A preferable solution would be to simply remove the >> ‘push/pull/removePlugin*’ stuff from our build system which is the >> reason why we are discussing keeping this “Github SVN” hack initially. >> >> IMO people can manage their plugins more conveniently by directly >> running the “installation” command of their choice ‘git clone’, ‘svn >> checkout’, ‘ln -s’, ... in their plugins directory without needing OFBiz >> build system to do it poorly for them. >> >>> Le 07/11/2019 à 14:00, Gil Portenseigne a écrit : >>>> Hello Deepak, all, >>>> >>>> I do not have a strong opinion about separating plugins into independent >>>> git repositories but here are my thought : >>>> >>>> Plugins integration in OFBiz is intended to be used with a maven >>>> repository that hosts the plugin releases for the users. See as a >>>> reference the ‘OFBiz Plugins tasks’ or README.adoc about ‘OFBiz plugin >>>> system’. >>>> >>>> We did not implemented an official maven repository for plugin releases, >>>> so there is no simple way to install a plugin without using VCS. There >>>> might be tasks to be done to have an nice answer to how an user can >>>> install a sole plugin from a downloaded release archive. >>>> > -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Git repo for each ofbiz plugin
Taher Alkhateeb writes: > I'm not sure I see this issue with the same perspective. Having the > build system automatically download (and run some code) for a certain > component is a convenience especially for the community components. > Things are even easier when using git as there are many plugins to > support that. I'm also not sure what is "poor" about that I agree that having the “build system” downloading things automatically is convenient and desirable. But IMO what we should mean by a build system is Gradle (or Maven, SBT, Leinigen, ...) not “OFBiz pretending to be a build system ontop of Gradle”. Let me clarify what I mean by “OFBiz build system doing things poorly”. There are a lot a important features that people would expect from a tool downloading/installing plugins (a.k.a package manager) that are missing like for example: - having a manifest listing the installed plugins with their version - removing unneeded dependencies when uninstalling a plugin, - having a way to upgrade an install plugin (automatically upgrading its dependencies if needed). - Ensuring that the plugin is compatible with the framework version - Resolve plugin version mismatchs consistently (A depends on B-0.1 and C depends on B-0.2 => use B-0.2) - Having a shared cache for already downloaded dependencies. One could then say “we just need to implement those features” to do things properly. But having some experience with in the domain of package managers, I know that things are too *complex* for us OFBiz developpers to mess with that. This is why I am proposing to remove the push/pull/remove commands and advocate towards working on the adoption of existing distribution interfaces like Jar libraries (See work done in OFBIZ-11161 [1]). [1] https://issues.apache.org/jira/browse/OFBIZ-11161 > On Mon, Nov 11, 2019 at 10:37 PM Mathieu Lirzin > wrote: >> >> Hello, >> >> Jacques Le Roux writes: >> >> > I propose that waiting for plugins Maven repos we simply continue to >> > use Svn through Github as I explained elsewhere >> >> No need to wait for Maven plugins before acting responsibly. :-) >> >> A preferable solution would be to simply remove the >> ‘push/pull/removePlugin*’ stuff from our build system which is the >> reason why we are discussing keeping this “Github SVN” hack initially. >> >> IMO people can manage their plugins more conveniently by directly >> running the “installation” command of their choice ‘git clone’, ‘svn >> checkout’, ‘ln -s’, ... in their plugins directory without needing OFBiz >> build system to do it poorly for them. >> >> > Le 07/11/2019 à 14:00, Gil Portenseigne a écrit : >> >> Hello Deepak, all, >> >> >> >> I do not have a strong opinion about separating plugins into independent >> >> git repositories but here are my thought : >> >> >> >> Plugins integration in OFBiz is intended to be used with a maven >> >> repository that hosts the plugin releases for the users. See as a >> >> reference the ‘OFBiz Plugins tasks’ or README.adoc about ‘OFBiz plugin >> >> system’. >> >> >> >> We did not implemented an official maven repository for plugin releases, >> >> so there is no simple way to install a plugin without using VCS. There >> >> might be tasks to be done to have an nice answer to how an user can >> >> install a sole plugin from a downloaded release archive. >> >> >> >> -- >> Mathieu Lirzin >> GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37 -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Git repo for each ofbiz plugin
Hello, Jacques Le Roux writes: > I propose that waiting for plugins Maven repos we simply continue to > use Svn through Github as I explained elsewhere No need to wait for Maven plugins before acting responsibly. :-) A preferable solution would be to simply remove the ‘push/pull/removePlugin*’ stuff from our build system which is the reason why we are discussing keeping this “Github SVN” hack initially. IMO people can manage their plugins more conveniently by directly running the “installation” command of their choice ‘git clone’, ‘svn checkout’, ‘ln -s’, ... in their plugins directory without needing OFBiz build system to do it poorly for them. > Le 07/11/2019 à 14:00, Gil Portenseigne a écrit : >> Hello Deepak, all, >> >> I do not have a strong opinion about separating plugins into independent >> git repositories but here are my thought : >> >> Plugins integration in OFBiz is intended to be used with a maven >> repository that hosts the plugin releases for the users. See as a >> reference the ‘OFBiz Plugins tasks’ or README.adoc about ‘OFBiz plugin >> system’. >> >> We did not implemented an official maven repository for plugin releases, >> so there is no simple way to install a plugin without using VCS. There >> might be tasks to be done to have an nice answer to how an user can >> install a sole plugin from a downloaded release archive. >> -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Toirtoisegit keeps adding "/gradle/" in .gitignore
Taher Alkhateeb writes: > as a general rule I would recommend against avoiding commits as a > workaround for specific tools. perhaps you can change the tool > settings to not generate this file. +1 > on another note, you might want to consider something like git-cola > [1]. I it manages to combine simplicity with richness. Or maybe it is just time to adopt a GNU/Linux distribution instead of fighting against an hostile developement environment. ;-) > [1] https://git-cola.github.io/ > > On Mon, Nov 11, 2019 at 3:50 PM Jacques Le Roux > wrote: >> >> Hi, >> >> As you may know I'm mostly on Windows and I use ToirtoiseGit to >> handle things, and in a lesser way Git Bash and eGit (Eclipse >> plugin). >> >> There is something which is annoying me with ToirtoiseGit. I don't >> know why but it keeps adding "/gradle/" in .gitignore. >> >> I want to push it, but before I'd like your opinions. >> >> Thanks >> >> Jacques >> -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: [ofbiz-framework] branch trunk updated: Fixed: (OFBIZ-)
Hello again,
‘./gradlew check’ fails after this commit, because it introduces some
linting issues.
> Task :checkstyleMain FAILED
--8<---cut here---start->8---
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at:
> file:///home/mthl/src/ofbiz/build/reports/checkstyle/main.html
Checkstyle files with violations: 1039
Checkstyle violations by severity: [error:37783]
--8<---cut here---end--->8---
jler...@apache.org writes:
[...]
> +
> +Map parameterMap =request.getParameterMap();
^ missing space
> +if (parameterMap != null) {
> +List params = new
> ArrayList();
> +request.getParameterMap().forEach((name, values) -> {
> +for(String value : values) {
^ missing space
> +params.add(new BasicNameValuePair(name, value));
> +}
> +});
> +String queryString = URLEncodedUtils.format(params,
> Charset.forName("UTF-8"));
> +uri = uri + "?" + queryString;
^ trailing whitespace
> + }
> +
^trailing whitespaces
Can you fix those ? :-)
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: [ofbiz-framework] branch trunk updated: Fixed: (OFBIZ-)
Hello Jacques,
jler...@apache.org writes:
> commit 5170e9d89503afa13d4ea1492b0ba73b2f92e528
> Author: Jacques Le Roux
> AuthorDate: Sat Nov 9 14:25:00 2019 +0100
>
> Fixed:
> (OFBIZ-)
>
> While working on OFBIZ-9804 (verification email for Newsletter) I was
> confronted
> with misc. issues. One of them was that SeoContextFilter.java was not
> handling
> query strings.
>
> This fixes it
> ---
> .../ofbiz/product/category/SeoContextFilter.java | 18
> +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git
> a/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java
>
> b/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java
> index 17ab0ae..b7cab04 100644
> ---
> a/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java
> +++
> b/applications/product/src/main/java/org/apache/ofbiz/product/category/SeoContextFilter.java
> @@ -96,6 +99,19 @@ public class SeoContextFilter implements Filter {
> HttpServletResponse httpResponse = (HttpServletResponse) response;
>
> String uri = httpRequest.getRequestURI();
> +
> +Map parameterMap =request.getParameterMap();
> +if (parameterMap != null) {
The documentation of ‘getParameterMap’ is not stating that the return
value can be ‘null’ contrary to what is done in other methods of the
same class so I guess we could assume that an empty map is returned.
> +List params = new
> ArrayList();
^^^
Unnecessary type
declaration
> +request.getParameterMap().forEach((name, values) -> {
> +for(String value : values) {
> +params.add(new BasicNameValuePair(name, value));
> +}
> +});
> +String queryString = URLEncodedUtils.format(params,
> Charset.forName("UTF-8"));
> +uri = uri + "?" + queryString;
> +}
Is there any reason why you are using ‘URLEncodedUtils.format’ instead
of simply retrieving the query string from the servlet request, like
this?
--8<---cut here---start->8---
String uri = httpRequest.getRequestURI();
String queryString = httpRequest.getQueryString();
if (queryString != null) {
uri = uri + "?" + queryString;
}
boolean forwarded = forwardUri(httpResponse, uri);
--8<---cut here---end--->8---
In order to help understand what is the expected behavior of
‘SeoContextFilter#doFilter’ and make the bug fix more future proof, it
would be really nice if you could add a unit test demonstrating how
query string examples should be handled by this fiter.
Thanks.
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: question about ServiceHandler.checkSecureParameter
Hello James, James Yong writes: > Understand the intent of checkSecureParameter function is to avoid sensitive > information > in the URL during POST method. A proposal is made to provide an > attribute (i.e. allow-query-string-for-service-event) to allow url > parameters / query string for certain request. Shouldn't the value for > this attribute be false, instead of true, when no value is specified > for the attribute? What would be required before discussing the details of the proposal is a detailed scenario demonstrating that in the context of OFBiz event handlers accepting query parameters from a HTTP request is less secure than accepting only body parameters. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Git repo for each ofbiz plugin
Hello Deepak, Deepak Dixit writes: > As we moved to from svn to git, so I think now its time to create new git > repository for each and indiviaual plugins instead of maintaing single > ofbiz-plugins.git > > It will help to manage the plugin life cycle properly, > if anyone want to use single or set of plugins they can easiliy setup with > the proposed way. > > If we create git repo for each plugins definately we have to manage the > dependencies and release properly, I am confident that as a community we > can manage this thing :) > > If this looks good, I'll initiate further process for the same. > > Please share your thoughts on this. I sympathise with the intent of having better modularity. However I think OFBiz plugin API is still in its infancy and should become more mature before considering moving in the direction of splitting every plugin we are maintaining into its own Version Control System (VCS) repository. Let me give you a concrete example demonstrating the kind of problems that splitting plugins into multiple VCS repositories will bring. One major limitation of the current OFBiz plugin API is that you cannot use Gradle plugins that depends on the java plugin (like the scala plugin for example) inside your local ‘build.gradle’ file without breaking the global build. Fixing that bug will likely mean introducing breaking changes to the OFBiz plugin API that would then need to be propagated in every plugin we maintain. With 2 VCS repositories (framework + plugins) things are already not ideal, but with ‘n’ VCS repositories a committer would need to create ‘n’ commits and users would need to run ‘git pull’ in ‘n’ repositories which is not manageable. Basically to mitigate this maintenance nightmare we would then need a custom meta VCS that would be reponsible of managing our set of VCS repositories to pull, commit, push... But do we really want that? Do we have the shoulders to build and maintain our custom package manager? I would say no. As a side note, one major reason for using a VCS is the ability to reproduce the state of a specific version when dealing with bugs (‘git bisect’ is meant to help in that regard). However when splitting a code base into multiple VCS repositories without having a versioned dependency manifest we simply loose such capability. Sorry for ruining the party. :-) -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: [ofbiz-framework] branch trunk updated: Fixed: content/control/WebSiteCms?webSiteId=CmsSite fails (OFBIZ-11266)
Hello Jacques,
jler...@apache.org writes:
> This is an automated email from the ASF dual-hosted git repository.
>
> jleroux pushed a commit to branch trunk
> in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
>
>
> The following commit(s) were added to refs/heads/trunk by this push:
> new 2af7119 Fixed: content/control/WebSiteCms?webSiteId=CmsSite fails
> (OFBIZ-11266)
> new 6f5924c Merge branch 'trunk' of
> https://gitbox.apache.org/repos/asf/ofbiz-framework into trunk
> 2af7119 is described below
In order to avoid unnecessary merge commits, I would recommend using
‘git pull --rebase’ instead of ‘git pull’
If you don't want to be bother with the extra ‘--rebase’ argument you
can configure git to automatically rebase instead of merge when doing
‘git pull’ with:
git config --global pull.rebase true
HTH,
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Upgrading Groovy 2.4.16 → 2.5.8
Thanks Gil, Committed in revision 1869136 (BTW I made a mistake by referencing OFBIZ-11268 instead of OFBIZ-11263). Gil Portenseigne writes: > I've test the upgrade and saw no issue, so +1. > > Gil > > > > Le 18:19 - dimanche 27 oct., Mathieu Lirzin a écrit : >> Hello, >> >> I have open OFBIZ-11263 [1] to upgrade Groovy to its latest stable >> release on ‘trunk’. >> >> I did not detect any issue with the upgrade so I intend to commit the >> patch in the following days. If you are aware of an issue please jump >> in. >> >> Thanks. >> >> [1] https://issues.apache.org/jira/browse/OFBIZ-11263 >> >> -- >> Mathieu Lirzin >> GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37 -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: svn commit: r1869039 - /ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ControlServlet.java
Hello Aditya, Aditya Sharma writes: > Just to clarify, as per commit message template document[1] the template is > > [Implemented|Improved|Fixed|Completed|Documented|Reverted]: [Jira > title|Free text] > > Can we use "Free text" instead of "Jira title" when committing specifically > for any Jira? Indeed there is a grammar ambiguity in the commit template. My personal interpretation was that using the “Jira title” was only for trivial stuff or when being uninspired. :-) In practice I always use "free text" to avoid having multiple commits with the same commit summary (e.g. first line) which IMO helps identifying commits when displaying a short version of the log history with: git log --oneline When using “Jira title” form we end up with something like Improved: OFBIz does not have *magic feature* (OFBIZ-X) Improved: OFBIz does not have *magic feature* (OFBIZ-X) Fixed: OFBIz does not have *magic feature* (OFBIZ-X) Improved: OFBIz does not have *magic feature* (OFBIZ-X) Implemented: OFBIz does not have *magic feature* (OFBIZ-X) Implemented: OFBIz does not have *magic feature* (OFBIZ-X) Where as when using the “free text” form we can get something more informative like Improved: Remove uneeded code in bar (OFBIZ-X) Improved: Refactor magic feature to avoid foo (OFBIZ-X) Fixed: Fix regression from magic feature (OFBIZ-X) Improved: Avoid unecessary null checks (OFBIZ-X) Implemented: Add new magic feature (OFBIZ-X) Implemented: Add new tests for module foo (OFBIZ-X) In any case if we collectively decide that the first form is preferable, I will conform to that guideline. Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Upgrading Groovy 2.4.16 → 2.5.8
Hello, I have open OFBIZ-11263 [1] to upgrade Groovy to its latest stable release on ‘trunk’. I did not detect any issue with the upgrade so I intend to commit the patch in the following days. If you are aware of an issue please jump in. Thanks. [1] https://issues.apache.org/jira/browse/OFBIZ-11263 -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Using ‘checkstyle’ linting tool
Hello Jacques, Jacques Le Roux writes: > Le 19/10/2019 à 17:05, Mathieu Lirzin a écrit : >> Hello Again, >> >> Taher Alkhateeb writes: >> >>> No opinion for or against, but I'd like to mention that the majority of our >>> code base is xml, and the emphasis is on switching to groovy for >>> everything, so the return on value from linting java might not be very >>> high, especially since our problems in java are not primarily formatting >>> issues, but rather design and architectural issues. >> Towards the goal of adding actual value via linting, I have just found >> CodeNarc [1] which provide a linting facility for Groovy and is easily >> available via a Gradle plugin [2]. >> >> It might be interested to set this up for OFBiz. >>[1] http://codenarc.sourceforge.net/ >> [2] https://docs.gradle.org/current/userguide/codenarc_plugin.html > > You are most welcome: https://issues.apache.org/jira/browse/OFBIZ-11167 :) Excellent, we now just need a volunteer to do the work. :-) -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: question about ServiceHandler.checkSecureParameter
Hello, Samuel writes: > Moreover if you don't use a service event in your request map you can > access whatever url parameter you want, so I cannot see why service > event is so particular in this regards. Indeed if the issue is about forbidding URI parameters because of security reason, this check should be hardcoded in the RequestHandler not by individual EventHandler implementations. Otherwise this is just absurd because every administrator/integrator can then implement an ad-hoc Java/Groovy event handler invoking a service without being warned about a “known” security issue. Jacques Le Roux writes: > I agree that it's an overkill. I guess because services give you more > power so need to be more secure. > > The same person that, like you, doubted about the reason of > checkSecureParameter() spoke also about the possibility to "inject > stuff using parameters" > > Here is what he said (actually this was reported to me by a 3rd person > but I much trust them both) > >"The other case I reported has more to do with people being able to >inject stuff using parameters. Because they are not always >escaped. In particular the case for the VIEW_INDEX parameter and >alike view_size, view_index often in combination with screen >widget but not only" > > Anyway what would you suggest? You know you can set > service.http.parameters.require.encrypted=N, is that not enough? I am not convinced by the explanation or by the non-solution of keeping an option with confusing security impacts. IMO for the sake of both simplicity and sanity we should just nuke the option and accept URI parameters unconditionally. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Backward compatibility for Java interfaces
Mathieu Lirzin writes: > However I have just discovered that in 2016 some refactoring work chose > a different approach by simply changing a method signature in the > ‘ContainerConfig’ interface [1]. [...] > [1] > https://github.com/apache/ofbiz-framework/commit/d44020f0d6b2af4c049722751e4586a3ae5b2d98#diff-af4722454ae770d57420936da9bd4e29L55-R56 For the record I have discovered a similar approach in 2012 [2]. [2] https://github.com/apache/ofbiz-framework/commit/c386b7fc9a8d61923efb9bbd2e472d1cac3d8a6f#diff-6a7b97cfa75c3d5f2ec74abbb2d42316L78-R78 -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Backward compatibility for Java interfaces
Hello, In my recent work on refactoring ‘Container’ and ‘ContainerConfig’ classes (See trunk) I have chosen (without discussing on ML) to use deprecation for stuff that were likely to be used by client code implementing additional containers in their plugins. However I have just discovered that in 2016 some refactoring work chose a different approach by simply changing a method signature in the ‘ContainerConfig’ interface [1]. Having @Deprecated methods is a tradeoff that provides smoother migration path for client code with the cost of cluttering our codebase which make things harder to maintain. So I would like to know what are the general expectations of people developing OFBiz plugins in term of backward compatibility and what people would recommend as a general guideline regarding “modifying public Java interfaces”. What do people think? [1] https://github.com/apache/ofbiz-framework/commit/d44020f0d6b2af4c049722751e4586a3ae5b2d98#diff-af4722454ae770d57420936da9bd4e29L55-R56 -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Migrating to Git
Hello, Swapnil M Mane writes: > As discussed previously, all the essential details are incorporated in this > document [1] to help us in migrating from SVN to Git. > Please have a look and let us know your kind feedback/thoughts. > Here are references [2] [3] to our previous discussions for your quick > reference. Thanks for thise reminder, I think this workflow document is sufficiently well thought to move forward. I have open the following ticket: https://issues.apache.org/jira/browse/INFRA-19308 Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Using ‘checkstyle’ linting tool
Hello Again, Taher Alkhateeb writes: > No opinion for or against, but I'd like to mention that the majority of our > code base is xml, and the emphasis is on switching to groovy for > everything, so the return on value from linting java might not be very > high, especially since our problems in java are not primarily formatting > issues, but rather design and architectural issues. Towards the goal of adding actual value via linting, I have just found CodeNarc [1] which provide a linting facility for Groovy and is easily available via a Gradle plugin [2]. It might be interested to set this up for OFBiz. [1] http://codenarc.sourceforge.net/ [2] https://docs.gradle.org/current/userguide/codenarc_plugin.html -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Migrating to Git
Now that the Git synchro is working again, I think it would be great if we could move forward regarding the SVN to Git migration. I am not sure what needs be done to achieve such result, is it just a matter of opening a INFRA ticket on Jira? What can I do to help? Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Using ‘checkstyle’ linting tool
Hello Taher, Taher Alkhateeb writes: > No opinion for or against, but I'd like to mention that the majority > of our code base is xml, and the emphasis is on switching to groovy > for everything. so the return on value from linting java might not be > very high, especially since our problems in java are not primarily > formatting issues, but rather design and architectural issues. Sure I agree that linting is not very important compared to design and architecture. What is nice about automating trivial stuff such as linting is that it allows to focus on the essential parts when doing code reviews and not on the irrelevant details (indentation, naming conventions, ...). > There are many great ideas for things to improve. The question is what to > prioritize and bring to the top of your to do list. The question I have is what is on top of your todo list Taher? :-) Thanks for your input. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Using ‘checkstyle’ linting tool
Hello, Linting [1] is a software engineering practice which make the code more readable and maintainable by improving its consistency and avoiding potential programming mistakes. Gradle provides a core plugin for the ‘checkstyle’ tool [2][3] which implement a linting facility for the Java language. I have opened OFBIZ-11251 [4] which proposes a patch for using this Gradle plugin in OFBiz. With this patch when contributors will write Java code that do not match the OFBiz coding convention [5] an error will be reported by ‘gradlew check’ target. However when hacking running ‘gradlew’, ‘gradlew run’ or ‘gradlew "ofbiz ..."’ will not bother integrators or contributors with any linting errors. The idea is to recommend contributors to run ‘gradlew check’ before sending a patch or committing one and maybe add a Buildbot task for running such verification and blaming the faulty committer. What do people think? If nobody oppose I will commit the OFBIZ-11251 patch in 3 days. [1] https://en.wikipedia.org/wiki/Lint_(software) [2] https://checkstyle.org/ [3] https://docs.gradle.org/current/userguide/checkstyle_plugin.html [4] https://issues.apache.org/jira/browse/OFBIZ-11251 [5] https://cwiki.apache.org/confluence/display/OFBIZ/Coding+Conventions -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Providing utilitaries for integration tests
Hello Jacques,
Jacques Le Roux writes:
> So we give up here, right?
I think providing helper methods for integration tests is a good long
term idea, So I don't think we should give up. :-)
> Le 08/10/2019 à 13:12, Jacques Le Roux a écrit :
>
>> Hi Mathieu
>>
>> I forgot to mention that I put this method in GroovyUtil.java.
>>
>> It was hastily done because doing so I rapidly thought that "Anyway it would
>> not add much, just a cover function"
>>
>> It works with a complete/correct version:
>>
>> public static Map testGroovy(Delegator delegator, LocalDispatcher
>> dispatcher, Map serviceCtx,
>> String serviceName) throws GenericEntityException,
>> GenericServiceException {
>> GenericValue userLogin = EntityQuery.use(delegator)
>> .from("UserLogin")
>> .where("userLoginId", "system")
>> .cache()
>> .queryOne();
>> serviceCtx.put("userLogin", userLogin);
>> return dispatcher.runSync(serviceName, serviceCtx);
>> }
>>
>> I still wonder if it of much use, ie compare:
>>
>> void testSendOrderChangeNotification() {
>> Map serviceCtx = [
>> orderId: 'TEST_DEMO10090',
>> sendTo: 'test_em...@example.com',
>> ]
>> Map serviceResult = GroovyUtil.testGroovy(delegator, dispatcher,
>> serviceCtx, 'sendOrderChangeNotification');
>> assert ServiceUtil.isSuccess(serviceResult)
>> assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
>> }
>>
>> with
>>
>> void testSendOrderChangeNotification() {
>> Map serviceCtx = [
>> orderId: 'TEST_DEMO10090',
>> sendTo: 'test_em...@example.com',
>> userLogin:
>> EntityQuery.use(delegator).from('UserLogin').where('userLoginId',
>> 'system').cache().queryOne()
>> ]
>> Map serviceResult =
>> dispatcher.runSync('sendOrderChangeNotification', serviceCtx)
>> assert ServiceUtil.isSuccess(serviceResult)
>> assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
>> }
>>
>> It's shorter and can be used for most Groovy tests. But you have to
>> pass the delegator, dispatcher and serviceCtx which are else
>> transparent. Not sure it's of better use.
>>
>> What do you think?
I agree that that having to pass the delegator, dispatcher and
serviceCtx is not ideal and tend make the test less clear.
In order to avoid repetitive code a nice first helper method would be
for example one for retrieving the default userLogin like what is done
in ‘QuoteTests.groovy’
// Retrieves a particular login record.
GenericValue getUserLogin(String userLoginId) {
GenericValue userLogin = EntityQuery.use(delegator)
.from('UserLogin').where(userLoginId: userLoginId).queryOne()
assert userLogin
return userLogin
}
We could even add a default login user.
// Retrieves the default login record.
GenericValue getUserLogin() {
return getUserLogin('system');
}
I guess we should add such method directly in the ‘OFBizTestCase’ class
to be able to reuse it in all test cases and avoid having to pass the
‘delegator’ and ‘dispatcher’ as method arguments.
The creation of the service input map of your example would look like
this:
void testSendOrderChangeNotification() {
Map serviceCtx = [
orderId: 'TEST_DEMO10090',
sendTo: 'test_em...@example.com',
userLogin: getUserLogin()
]
Map serviceResult = dispatcher.runSync('sendOrderChangeNotification',
serviceCtx)
assert ServiceUtil.isSuccess(serviceResult)
assert serviceResult.emailType.equals("PRDS_ODR_CHANGE")
}
In any case I think that finding the generic and reusable helper methods
can be done incrementally.
What do you think?
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: buildbot failure in on ofbizTrunkFramework
Hello Jacques, Jacques Le Roux writes: > Actually the problem is more that we have 7 references to > > bodyScreenLocation="component://ecommerce/widget/EmailOrderScreens > > in applications (OrderDemoData.xml and OrderTestData.xml) > > I see 2 options: > > 1. Move OrderNotificationTests.groovy from order to ecommerce > 2. Move EmailOrderScreens from ecommerce to order > > I don't know which one would be easier and I'm not sure yet which one > would be better from a disentanglement perspective. > > Which one would you prefer an why? I hope answers in a week, w/o any I'll > pick one. IMO moving the tests to the ecommerce plugin is a preferable option. Thanks for investigating. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Providing utilitaries for integration tests (was: svn commit: r1867889 …)
Hello Jacques,
Jacques Le Roux writes:
> I had a look at this idea but I finally gave up. I thought about creating
>
> public static Map testGroovy(Map paramNames, String
> serviceName) {
> userLogin =
> EntityQuery.use(delegator).from('UserLogin').where('userLoginId',
> 'system').cache().queryOne()
> Map serviceCtx = paramNames.put("userLogin",
> EntityQuery.use(delegator).from('UserLogin').where('userLoginId',
> 'system').cache().queryOne())
> Map result = dispatcher.runSync(serviceName, serviceCtx)
> }
>
> And only pass params and service name, but EntityQuery is not yet available
> in base component when compiling.
What do you mean by “EntityQuery is not yet available in base component
when compiling”?
Can you provide a patch and the command you run to get to this failing
scenario?
> Anyway it would add much, just a cover function
>
> Le 03/10/2019 à 14:29, Jacques Le Roux a écrit :
>> BTW looking at OrderTests.groovy it seems we can refactor Groovy tests, a
>> lot of is common...
>>
--
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Refactoring ServiceSemaphore (OFBIZ-11204)
Hello Gil, Gil Portenseigne writes: > In one of our customer project, where we are using multiple OFBiz > instances to run backend jobs, in which some are using service semaphore > system. > > While these semaphore jobs are run quite intensively by several OFBiz > process, we stumble upon a lot of 'Error' logs that are polluting other > production log, in the fact that these are not really error, but a > failure trying to insert a lock into ServiceSemaphore table, that leads > to another waiting loop. > > I then proposed a refactoring of the implementation (jira OFBIZ-11204), > that works currently fine in our production environment. > > Since semaphore feature is a quite critical one, some reviews would be > appreciated before commiting it in the codebase. Thanks for tackling this issue and providing some documentation I am unfamiliar with this part of the OFBiz internals and fails to properly understand it properly after a quick glance. The only remark I can make right right now is that the terminology used seems confusing, which is not directly related to your patch but to the current implementation. A semaphore is a concurrency object/variable allowing an arbitrary bounded number of "processors" to run concurrently in some critical section. However the ‘ServiceSemaphore’ seems to allow only one processor to execute a service at a time, which makes it a “Lock” instead of a general “Semaphore”. So this not really a priority, but it might be a good idea to later rename this class to ‘ServiceLock’ and make it implement the ‘java.util.concurrent.locks.Lock’ interface for more understandeable/predictable semantics. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: buildbot failure in on ofbizTrunkFramework
It Works locally, build...@apache.org writes: > The Buildbot has detected a new failure on builder ofbizTrunkFramework while > building . Full details are available at: > https://ci.apache.org/builders/ofbizTrunkFramework/builds/1119 > > Buildbot URL: https://ci.apache.org/ > > Buildslave for this Build: asf946_ubuntu > > Build Reason: The AnyBranchScheduler scheduler named 'onTrunkFrameworkCommit' > triggered this build > Build Source Stamp: [branch ofbiz/ofbiz-framework/trunk] 1868030 > Blamelist: mthl > > BUILD FAILED: failed shell_2 > > Sincerely, > -The Buildbot > > > -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml
Hello Jacques, Jacques Le Roux writes: > Le 02/10/2019 à 17:21, Mathieu Lirzin a écrit : > >> jler...@apache.org writes: >> >>> Author: jleroux >>> Date: Wed Oct 2 14:46:00 2019 >>> New Revision: 1867889 >>> >>> URL: http://svn.apache.org/viewvc?rev=1867889&view=rev >>> Log: >>> Improved: Unit test case for service - SendOrderBackorderNotification >>> (OFBIZ-8810)(OFBIZ-9647)(OFBIZ-9671) >>> >>> While working on this I stumbled upon an issue related with >>> webSiteId="OrderEntry" well related by Ratnesh Upadhyay in OFBIZ-9647. >>> >>> Unlike him I decided not to remove the webSiteId="OrderEntry" entries but to >>> replace them by webSiteId="WebStore" >>> >>> Added: >>> >>> ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml >>> (with props) >> It would be nice if you could rewrite this "integration" test in Groovy >> or Java. > > Yes of course (Groovy preferably IMO) > > >> >> If I remember correctly we have decided to not accept new minilang code >> in our codebase. Am I overlooking something? >> >>> Modified: >>> >>> ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml >>> ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml >>> ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml >>> >>> ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml >> Thanks. >> > > We have discussed this already and here is a special case as commented at > bottom of OFBIZ-1463 and at https://markmail.org/message/ute4ulnetz2h4ed5 > > I don't want to lose the work embedded in this and 15 others patches > available at OFBIZ-1463 (to be checked one by one). > > Integration tests are data driven, I mean that when you have defined the data > the test itself mostly follows. > Most of the time it's not a big deal to switch the test from Minilang to > Groovy. > I/we can do it after a 1st pass where we value the current patches (already > old for 4 years) > > BTW, working on this one I discovered an issue with > <>. It was not really part of the patch, but the > patch revealed it. > Hence (OFBIZ-9647)(OFBIZ-9671) also in this improvement, almost a bug > actually for these 2 Jiras. It was 90% of the time I passed on this commit. > It's incidental but part of the process and I don't want to loose the efforts > put in the remaining available patches. > It's also a matter of not overloading my mind, step by step, if you want. > > So I'll continue to check these patches, apply them and later migrate tests > to Groovy. > Then I expect the tests will have been validated and it will be almost > mechanical to migrate them. > > But you are right and I'll open new Jiras for migrating them, knowing > that we have a 1300+ others tests to migrate[1]! (ie here it's 1%+, > and I expect simple ones :)) Sorry, but I still don't buy your arguments. :-) I will repeat what I said in the previous discussion: * If this is not a big deal to migrate integration tests *why don't you just™* do the migration work before committing those patches? * If you don't have time to do it right now, chances are that you won't have time for it later, so adding more minilang simply means more burden on others. I would prefer that we settle this disagreement before you go ahead. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: svn commit: r1867889 - in /ofbiz/ofbiz-framework/trunk/applications: datamodel/data/demo/OrderDemoData.xml order/minilang/test/OrderTest.xml order/servicedef/secas.xml order/testdef/OrderTest.xml
Hello Jacques, jler...@apache.org writes: > Author: jleroux > Date: Wed Oct 2 14:46:00 2019 > New Revision: 1867889 > > URL: http://svn.apache.org/viewvc?rev=1867889&view=rev > Log: > Improved: Unit test case for service - SendOrderBackorderNotification > (OFBIZ-8810)(OFBIZ-9647)(OFBIZ-9671) > > While working on this I stumbled upon an issue related with > webSiteId="OrderEntry" well related by Ratnesh Upadhyay in OFBIZ-9647. > > Unlike him I decided not to remove the webSiteId="OrderEntry" entries but to > replace them by webSiteId="WebStore" > > Added: > > ofbiz/ofbiz-framework/trunk/applications/order/minilang/test/OrderTest.xml > (with props) It would be nice if you could rewrite this "integration" test in Groovy or Java. If I remember correctly we have decided to not accept new minilang code in our codebase. Am I overlooking something? > Modified: > > ofbiz/ofbiz-framework/trunk/applications/datamodel/data/demo/OrderDemoData.xml > ofbiz/ofbiz-framework/trunk/applications/order/servicedef/secas.xml > ofbiz/ofbiz-framework/trunk/applications/order/testdef/OrderTest.xml > > ofbiz/ofbiz-framework/trunk/applications/order/testdef/data/OrderTestData.xml Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Adding a CONTRIBUTING.adoc file to the “ofbiz-framework” repository
Hello Jacques, Jacques Le Roux writes: > As Scott once said all could be resolved to improvement (I could not > find the reference but I clearly remember it) It depends if you believe in progress or not. :-) If not, every ticket/patch could be seen as “evolution” without taking side regarding if it improves or decays the current version. > BTW, if you have time and want some "entertainment" you might read > https://markmail.org/message/pgcf6zfqf6xkfmii > > Not really helping :) Having more context on a subject is always helpful. Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Adding a CONTRIBUTING.adoc file to the “ofbiz-framework” repository
Hello Pierre, Pierre Smits writes: > I am at a loss here, are you suggesting to introduce new ticket types, > and/or sub types? My intent was not to propose a change besides documenting things in a “CONTRIBUTING.adoc” file instead of in Confluence. I was trying to make more explicit the relation between the “Jira ticket types” and “commit prefixes” as there are currently documented [1][2] and used in practice. Here are the two lists: - Commit prefixes := Implemented|Improved|Fixed|Completed|Documented|Reverted - Jira ticket type := Improvement|Bug|New feature|Test|Wish|Task It is possible that I misinterpreted the current documentation or took inspiration from “mistakes” made by me or others. So the rules I stated in my previous mail may not reflect what we are supposed to do. > As far as JIRA shows there are only: > >1. bug (which can get to final successful resolution 'fixed') >2. improvement (which can get to final successful resolution >'implemented') >3. new feature (which also can get to final successful resolution >'implemented') >4. task (which can get to final successful resolution 'done') >5. test (which can get to a final successful resolution 'executed') >6. wish () > > Refactoring is not a recognised type, and IMO it should not be in. We > should keep things as simple as possible. > > Doesn't the proposed change for bug tickets (being able to classify it as > 'improved' confuse many? Or am I missing the point? I agree that our commit guidelines are complex for reasons or benefits that are not obvious to me, but that's another topic since I didn't proposed to change them. :-) I hope this message will help you understand the intent of my previous one. Thanks. [1] https://cwiki.apache.org/confluence/display/OFBIZ/OFBiz+commit+message+template [2] https://cwiki.apache.org/confluence/display/OFBIZ/OFBiz+Committers+Roles+and+Responsibilities -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Adding a CONTRIBUTING.adoc file to the “ofbiz-framework” repository (was: svn commit: r1867663 …)
Hello, Pierre Smits writes: > Fyi: improvement (tickets) get ‘implemented’, and bugs get ‘fixed’. As per > established conventions. >From what I understood from the examples and common practice [1], this is only partially true. Improvement tickets can be associated with both ‘Implemented:’ and ‘Improved:’ commits depending on the type of improvement: - Refactoring => “Improved:” - New feature => “Implemented:” [1] https://cwiki.apache.org/confluence/display/OFBIZ/OFBiz+commit+message+template > On Sat, 28 Sep 2019 at 15:50 Swapnil M Mane wrote: > >> Hi Deepak, >> >> Happy to see your commits in action. :) >> Just a minor suggestion, we put Jira ticket ID in separate line in commit >> log. >> Also, add colon ':' in Thanks statement. >> And since the ticket type is 'Improvement', it seems to me, we should use >> 'Improved' instead of 'Fixed'. >> >> Here is commit template for your quick reference >> >> https://cwiki.apache.org/confluence/display/OFBIZ/OFBiz+commit+message+template >> >> Following commit log template will help us in producing the monthly blog >> development details. As a general rule, I would say that working on a “bug” ticket implies at least one “Fixed:” commit, but it is possible to associate a complementary refactoring “Improved:” commit to a “bug” ticket. On the other hand an “improvement” ticket can not be associated with a “Fixed:” commit. What about adding a CONTRIBUTING.adoc file the repository stating those rules? This would make things far more visible and explicit than on Confluence which is not very visible (I often to keep bookmarks to retrieve some information) and far from the code. What do people think? -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Avoiding to log Uel stacktraces (was: svn commit: r1867635 - /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/string/UelFunctions.java)
Hello Gil, p...@apache.org writes: > Author: pgil > Date: Fri Sep 27 13:52:43 2019 > New Revision: 1867635 > > URL: http://svn.apache.org/viewvc?rev=1867635&view=rev > Log: > Improved: Refactor UelFunctions.java to remove error management via Exception > (OFBIZ-11213) > > This refacto was done to avoid logging stackTrace exception, when it is > possible > to manage it without exception. The "avoid logging stackTrace exception" part of your commit message gives the impression that the motivation for this rewrite is to hide programming errors inside Uel and not necessarily to make the implementation more understandeable. Can you give more context regarding this issue of unwanted/unnecessary logged stacktraces and explain why you choose the solution of silently hiding the "stacktrace" instead of fixing the code inside the Uel? Thanks. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: buildbot failure in on ofbizTrunkFrameworkPlugins
build...@apache.org writes: > The Buildbot has detected a new failure on builder ofbizTrunkFrameworkPlugins > while building . Full details are available at: > https://ci.apache.org/builders/ofbizTrunkFrameworkPlugins/builds/988 > > Buildbot URL: https://ci.apache.org/ > > Buildslave for this Build: asf945_ubuntu > > Build Reason: downstream > Build Source Stamp: [branch ofbiz/ofbiz-framework/trunk] 1867343 > Blamelist: mthl > > BUILD FAILED: failed shell_4 ‘gradlew testIntegration’ works locally with official plugins. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Issue with the synchronization of the Git mirror
Hello Jacques, Jacques Le Roux writes: > Found it: https://blogs.apache.org/infra/entry/subversion-to-git-service-git Thanks for the info. > Le 15/09/2019 à 15:38, Jacques Le Roux a écrit : >> Hi Mathieu, >> >> You have to wait. There has been a catastrophic crash and it's (very) slowly >> recovering >> >> Infra has restarted it, we can't help, , sorry for that >> >> Jacques >> >> Le 15/09/2019 à 14:35, Mathieu Lirzin a écrit : >>> Hello, >>> >>> The synchronization of the Git mirror for the “ofbiz-framework” >>> repository [1] is not working anymore which is making my contributor >>> life really difficult. :-) >>> >>> Does anyhow know how to fix this? what can I do to help? >>> >>> Thanks. >>> >>> [1] https://github.com/apache/ofbiz-framework >>> >> > -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Issue with the synchronization of the Git mirror
Hello, The synchronization of the Git mirror for the “ofbiz-framework” repository [1] is not working anymore which is making my contributor life really difficult. :-) Does anyhow know how to fix this? what can I do to help? Thanks. [1] https://github.com/apache/ofbiz-framework -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: svn commit: r1866545 - in /ofbiz/site/dtds: entitymodel.xsd services.xsd test-suite.xsd widget-form.xsd
Jacques Le Roux writes: > Le 07/09/2019 à 18:25, Mathieu Lirzin a écrit : >> Hello Jacques, >> >> jler...@apache.org writes: >> >>> Modified: ofbiz/site/dtds/services.xsd >>> URL: >>> http://svn.apache.org/viewvc/ofbiz/site/dtds/services.xsd?rev=1866545&r1=1866544&r2=1866545&view=diff >>> == >>> --- ofbiz/site/dtds/services.xsd (original) >>> +++ ofbiz/site/dtds/services.xsd Sat Sep 7 06:28:44 2019 >>> @@ -153,12 +153,28 @@ under the License. >>> >>> >>> >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> + >>> +If set to false, when the >>> permissions failed return the failMessage as error, else continue the >>> service and give the hand to origin service to resolve the >>> problem. >>> + >>> + >>> + >>> + >>> + >>> + >>> + >> Maybe we should use ‘xs:boolean’ instead? > Right Mathieu, done Thanks Jacques. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: svn commit: r1866559 - /ofbiz/ofbiz-framework/trunk/framework/service/src/main/java/org/apache/ofbiz/service/engine/EntityAutoEngine.java
Hello Nico, nma...@apache.org writes: > URL: http://svn.apache.org/viewvc?rev=1866559&view=rev > Log: > Improved: Manage itemStatusId and oldItemStatusId on entity-auto engine > (OFBIZ-11183) > Currently the entity auto engine manage the status change operation on an > Entity > during an update, with analyse the field statusId as new status and compare > with current > value through StatusValidChange system. If the change is validated, the > previous status > is returned in oldStatusId service parameter. > > Service definition example : > engine=entity-auto invoke=update auth=true> > Update an existing requirement > > > > > > I extend this process to an other standard status field: itemStatusId and > oldItemStatusId, often present on item element > engine=entity-auto invoke=update auth=true> > Update PicklistItem > > > > > > To realize this, I convert all call on statusId and oldStatusId raw naming by > a dynamic resolution field name resolution. > Like this the logical for statusId or itemStatusId are exactly the same The oldStatusId/itemStatusId/oldItemStatusId thing sounds like an advanced feature of the “entity-auto” engine that would be nice to document somewhere in order to let others know that it exists. Maybe you could write some line the “Developer manual” about that? :-) Thanks, -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: svn commit: r1866545 - in /ofbiz/site/dtds: entitymodel.xsd services.xsd test-suite.xsd widget-form.xsd
Hello Jacques, jler...@apache.org writes: > Modified: ofbiz/site/dtds/services.xsd > URL: > http://svn.apache.org/viewvc/ofbiz/site/dtds/services.xsd?rev=1866545&r1=1866544&r2=1866545&view=diff > == > --- ofbiz/site/dtds/services.xsd (original) > +++ ofbiz/site/dtds/services.xsd Sat Sep 7 06:28:44 2019 > @@ -153,12 +153,28 @@ under the License. > > > > + > + > + > + > + > + > + > + > + > +If set to false, when the > permissions failed return the failMessage as error, else continue the service > and give the hand to origin service to resolve the > problem. > + > + > + > + > + > + > + Maybe we should use ‘xs:boolean’ instead? -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: ASL2 header for meta-data files
Jacques Le Roux writes: > Le 03/09/2019 à 17:28, Jacques Le Roux a écrit : >>> That file is containing a line that excludes the meta-data files from >>> their previous location “**/framework/base/src/main/java/META-INF/**” My >>> commit moved those files to >>> “**/framework/base/src/main/resources/META-INF/**”, so IMO we should >>> adapt the exclusion line. >> >> Well spotted, indeed that's the right solution > > Done: I changed to the right location, that's all Thanks you Jacques, -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: ASL2 header for meta-data files
Jacques Le Roux writes: > Le 03/09/2019 à 15:32, Mathieu Lirzin a écrit : >> Hello Jacques, >> >> >>> Le 26/08/2019 à 13:47, Jacques Le Roux a écrit : >>>> If we need the stuff like >>>> >>>> org.apache.ofbiz.base.util.test.UtilObjectTests$TestFactoryIntf >>>> >>>> Then this one misses an ASL2 header >>>> >>>> I was also wondering how they are updated, when compiling? If so how is >>>> the ASL2 header added? >> Those files are meta-data files that are maintained manually. They are >> distributed inside the JAR to allow the ‘ServiceLoader’ API [1] to find >> the classes implementing a particular interface (corresponding to the >> file name) efficiently. >> [1]https://docs.oracle.com/javase/8/docs/api/java/util/ServiceLoader.html >> >> According to the specification, those files can include comments using >> the # character so technically we could add the ASL2 header, however I >> am not sure if we should/must copyright them. >> >> What are the recommandation of the ASF regarding meta-data or >> configuration files? > > Reading https://www.apache.org/legal/src-headers.html#faq-siteindocs and > sequel, specifically: > >/"If in doubt about the extent of the file's creativity, add the license > header to the file."/ After reading that, I would consider those files as “no creativity without doubt”. > It seems to me that all these files should have an ASL2 header. > All similar others have it so I guess it's easier to put it in this one and > go ahead. Else we would need to put these files as exceptions in > > https://svn.apache.org/repos/asf/ofbiz/tools/rat-excludes.txt That file is containing a line that excludes the meta-data files from their previous location “**/framework/base/src/main/java/META-INF/**” My commit moved those files to “**/framework/base/src/main/resources/META-INF/**”, so IMO we should adapt the exclusion line. I see that some “*.xsd” are excluded which seems undesirable since writing an XML schema is most of the time creative work. As a side note, I would highly recommend merging the "tools" repository inside “ofbiz-framework” repository to avoid issues like this one and help coordinating the maintenance of those tools. What do you think? -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
ASL2 header for meta-data files (was: svn commit: r1865717 ...)
Hello Jacques, Jacques Le Roux writes: > Any chances? Sorry for the delay, > Le 26/08/2019 à 13:47, Jacques Le Roux a écrit : >> >> If we need the stuff like >> >> org.apache.ofbiz.base.util.test.UtilObjectTests$TestFactoryIntf >> >> Then this one misses an ASL2 header >> >> I was also wondering how they are updated, when compiling? If so how is the >> ASL2 header added? Those files are meta-data files that are maintained manually. They are distributed inside the JAR to allow the ‘ServiceLoader’ API [1] to find the classes implementing a particular interface (corresponding to the file name) efficiently. According to the specification, those files can include comments using the # character so technically we could add the ASL2 header, however I am not sure if we should/must copyright them. What are the recommandation of the ASF regarding meta-data or configuration files? >> Le 22/08/2019 à 22:38, m...@apache.org a écrit : >>> Author: mthl >>> Date: Thu Aug 22 20:38:01 2019 >>> New Revision: 1865717 >>> >>> URL: http://svn.apache.org/viewvc?rev=1865717&view=rev >>> Log: >>> Improved: Separate resources from Java source files >>> (OFBIZ-11161) >>> >>> This moves the resource files in a dedicated "src/main/resources" >>> directory. This convention follows the Maven standard directory >>> layout which is the convention used by default in Gradle. >>> >>> [...] >>> Added: >>> ofbiz/ofbiz-framework/trunk/framework/base/src/main/resources/META-INF/services/org.apache.ofbiz.base.util.test.UtilObjectTests$TestFactoryIntf >>> URL: >>> http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/resources/META-INF/services/org.apache.ofbiz.base.util.test.UtilObjectTests%24TestFactoryIntf?rev=1865717&view=auto >>> == >>> --- >>> ofbiz/ofbiz-framework/trunk/framework/base/src/main/resources/META-INF/services/org.apache.ofbiz.base.util.test.UtilObjectTests$TestFactoryIntf >>> (added) >>> +++ >>> ofbiz/ofbiz-framework/trunk/framework/base/src/main/resources/META-INF/services/org.apache.ofbiz.base.util.test.UtilObjectTests$TestFactoryIntf >>> Thu Aug 22 20:38:01 2019 >>> @@ -0,0 +1,2 @@ >>> +org.apache.ofbiz.base.util.test.UtilObjectTests$FirstTestFactory >>> +org.apache.ofbiz.base.util.test.UtilObjectTests$SecondTestFactory >>> [...] [1] https://docs.oracle.com/javase/8/docs/api/java/util/ServiceLoader.html -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: [GRADLE] "CreateProcess error=206, The filename or extension is too long"
Hello Jacques, Jacques Le Roux writes: > I stumbled upon that this mrening. I tried all (reboot, clean, "Gradle > --stop", etc.) only > https://github.com/viswaramamoorthy/gradle-util-plugins worked you mean that Gradle refuses to start? > It seems I'll (we will) need it now due to recent changes in > build.gradle. I'll wait to confirm before creating a Jira and > committing... It might be related to the recent move of resources out of “/src/main/java/” to the standard “/src/main/resources/”. I think it keeping this separation is important, so adding this ‘gradle-util-plugins’ fixing this Windows specific issue might be a better solution IMO. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
Re: Should we remove modifiers when default (no modifier) is the same?
Hello Jacques, Jacques Le Roux writes: > I was looking at the work done by Pradhan Yash Sharma for OFBIZ-10477 "Parent > ticket for reducing scope of variables and methods" > > This work is related to > https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html > > I only applied the patch from OFBIZ-10478 "Reducing scope of variables in > org.apache.ofbiz.base package" > > Some changes are welcome, to improve security, like replacing public or > protected by private when possible > > I think removing public modifiers is also a good thing > > What do you think? I like the idea. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
