[GitHub] jena issue #436: JENA-1556 implementation
Github user afs commented on the issue: https://github.com/apache/jena/pull/436 Thanks - it's only being near to a release that makes it of significance. ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user xristy commented on the issue: https://github.com/apache/jena/pull/436 I haven't updated the documentation yet - which I plan to do over the next couple of days. I'm happy to mark [JENA-1556](https://issues.apache.org/jira/browse/JENA-1556) resolved now. ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user afs commented on the issue: https://github.com/apache/jena/pull/436 Staging now has links on the "Contributing" page to the workflow and to the release process. One last thing : can [Jena-1556](https://issues.apache.org/jira/browse/JENA-1556) be resolved or do you want to keep it open for something? If it is marked "resolved", the during release it will be closed. This helps tracking all the changes in a release. ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user xristy commented on the issue: https://github.com/apache/jena/pull/436 Thanks everyone. I seemed to have finally merged PR#436. I ended up using @rvesse plus a bit from the commit workflow (which I agree should be linked on the Contributing page): > git checkout master > git pull apache master > git merge Jena-1556-MutilingualEnhancements-3.8.0 > mvn clean install -Pdev > git push apache master I put the proper defns in the jena/.git/config for the remotes ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user kinow commented on the issue: https://github.com/apache/jena/pull/436 The docs look really good! Only feedback I have: * It's possible to avoid the empty commit, by amending the last commit adding "this closes #123". That's what I normally do when I work in other projects. * Should we also merge + --no-ff, or is fetch/rebase all right too? I've done it a few times, but happy to do merge + --no-ff instead if desirable * Perhaps a link in the website for it, under Contributing? Or a new page? It's probably related to http://jena.apache.org/getting_involved/reviewing_contributions.html too? (or maybe there's already a link somewhere?) Thanks ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user afs commented on the issue: https://github.com/apache/jena/pull/436 I have a copy of Jena cloned from git-wip-us.apache.org = a added second remote for github (this is so it is called as "github", not the full URL "https://github.com/apache/jena.git;). To merge, I have been using that one, merge from github (--no-ff) into local master and push to origin/master. Is there anything we can do to improve https://cwiki.apache.org/confluence/display/JENA/Commit+Workflow+for+Github-ASF? ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user afs commented on the issue: https://github.com/apache/jena/pull/436 Committer workflow: https://cwiki.apache.org/confluence/display/JENA/Commit+Workflow+for+Github-ASF ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user rvesse commented on the issue: https://github.com/apache/jena/pull/436 @xristy I think you can just merge as you would normally though you probably need to be explicit about the remote and branch you are using for remote operations: ``` > git checkout master > git pull apache-origin master > git merge Jena-1556-MutilingualEnhancements-3.8.0 > git push apache-origin master ``` ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user xristy commented on the issue: https://github.com/apache/jena/pull/436 @kinow Thank you very much for the pointers. I think I'm closer. I added a remote apache-origin (should have called it jena-origin I suppose). Then did: git push apache-origin and got encouraging result: Counting objects: 3316, done. Delta compression using up to 8 threads. Compressing objects: 100% (1017/1017), done. Writing objects: 100% (3316/3316), 795.73 KiB | 99.47 MiB/s, done. Total 3316 (delta 1505), reused 3218 (delta 1453) remote: jena git commit: added auxIndex unit test remote: jena git commit: added searchFor unit tests remote: jena git commit: various cleanup per @kinow remote: jena git commit: cleanup per comments from afs remote: jena git commit: JENA-1556 implementation To https://git-wip-us.apache.org/repos/asf/jena.git * [new branch]JENA-1556-MutilingualEnhancements-3.8.0 -> JENA-1556-MutilingualEnhancements-3.8.0 unfortunately I don't know how to merge the new branch into `master`. This is my first time working directly with the Apache git repo. ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user kinow commented on the issue: https://github.com/apache/jena/pull/436 @xristy if you'd like to merge it, and you cannot use the GitHub interface, then you can * Add a remote in your git repository using the developerConnection URL from the pom.xml (https://github.com/apache/jena/blob/master/pom.xml) but without the scm:git part (i.e. https://git-wip-us.apache.org/repos/asf/jena.git) * Merge the work and push to master * If the commit tree is OK, GitHub should understand this branch was merged. But if you changed or didn't rebase the code, you can still close it manually, or amend a commit and add somewhere in the message "This closes pr#436" HTH ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user xristy commented on the issue: https://github.com/apache/jena/pull/436 @afs I don't have write access to this github mirror repo and don't immediately see how work in the `git-wip-us.apache.org` so I'm not sure how to proceed with merging ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user afs commented on the issue: https://github.com/apache/jena/pull/436 Just checking in - when this is ready, merge it in. I don't have an opinion on @kinow's comments - I don't work closely with this area of the code - but I would say that simplifying/refactoring can happen as a second PR. ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user xristy commented on the issue: https://github.com/apache/jena/pull/436 @kinow I think the configuration and results reflect the `text:searchFor` functionality; however, in the analyzer defn for the tag `jp`: text:analyzer [ a text:GenericAnalyzer ; text:class "org.apache.lucene.analysis.ja.JapaneseAnalyzer" ; text:tokenizer <#tokenizer> ; ] the `text:tokenizer <#tokenizer> ;` is not effective. Tokenizer specs work with `ConfigurableAnalyzer` and are ignored in `text:GenericAnalyzer`. Perhaps a warning should be logged but that means checking for the presence of unsupported predicates? Re: > the complexity put on TextIndexLucene. A few methods are getting a boolean flag to change their behaviour. And when that happens too much, sometimes it may feel like the method has two behaviours, and writing tests or changing it may be challenging. Maybe it could extend it in some other way. I'm not sure how to improve this. The flag in `highlightResults` affects the value of the `effectiveField` in the context of a larger method, and the flag in `getQueryAnalyzer` conditions whether any useful work is done or not. I factored that as a method rather than leaving it inline in `query$` to reduce the clutter in that principal routine. Re: > it's not a batteries-included feature, if I understand correctly. You still need to prepare the other part of the solution, be it a tokenizer that gets a value such as "kinou", then searches some dictionary, and finally create tokens for :ex3 dc:title "æ¨æ¥" and "ãã®ã", or change the data a bit. Maybe this could be a separate project, or an extension of sorts. I'm not sure what you are recommending here. The `text:searchFor` and `text:auxIndex` functionalities are ways of configuring the _application_ of appropriate analyzers that have been separately defined. So yes the features are not self-contained in that analyzers do have to be supplied. ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user xristy commented on the issue: https://github.com/apache/jena/pull/436 @afs I will work on unit tests over the next couple of days. I wanted to get the main code in for review. It would be great to get into 3.8.0 pending @kinow and others reviews. ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user kinow commented on the issue: https://github.com/apache/jena/pull/436 This weekend I'm working on Commons, but planning on trying this pull request if time allows, so hopefully providing feedback if that worked with the Japanese example discussed in JIRA soon. ---
[GitHub] jena issue #436: JENA-1556 implementation
Github user afs commented on the issue: https://github.com/apache/jena/pull/436 Much cleaner! I don't see tests. Do you want to get this into the 3.8.0 release? I'm happy to hold off the release for a short while so this PR can be discussed and merged. (I'd like to get 3.8.0 done this month but we don't have a hard deadline.) @xristy What's your guidance here? ---
