#bookbrainz

/

      • elgranRoble has quit
      • elgranRoble joined the channel
      • elgranRoble has quit
      • riksucks has quit
      • riksucks joined the channel
      • riksucks has left the channel
      • riksucks joined the channel
      • CatQuest has left the channel
      • CatQuest joined the channel
      • ZaphodBeeblebrox joined the channel
      • Shubh
        monkey: Hi!
      • monkey
        Hello Shubh !
      • Feeling any better?
      • Shubh
        Yep!
      • monkey
        Sorry I forgot to respond to your last message. I didn't quite understand the whole thing. Currently we always have an empty row in the AC editor which makes things more complicated than they need to be, is that the issue?
      • Shubh
        We do have single row when AC is not disable for inline ac component
      • monkey
        By inline AC component, do you mean this search input? https://usercontent.irccloud-cdn.com/file/pNOKH...
      • I do see that even when empty, the state has an empty AC row: https://usercontent.irccloud-cdn.com/file/SWoYM...
      • (rather than nothing at all)
      • In which case I agree, we don't need an empty row and we can just add it when required
      • Shubh
        Would it be good idea to do this in separate PR?
      • monkey
        Probably best, yes
      • I was thinking the same
      • Let's focus on finishing the open PRs first
      • Shubh
        Yep!
      • monkey: i made the changes on processing entity as we discussed, let me know if there's any thing left in the early submit PR.
      • monkey
        👍
      • Shubh
        also that relationship merge issue should be fix after concurrent rel PR.
      • monkey
        Yep. So the order in which I should merge then? 874, then 876, then update the new-creation-form feature branch?
      • Shubh
        if possible merge #876 (concurrent rel) on new-creation-form branch then i will merge it with early-submit branch and will make some changes to support new relationship workflow. then we can merge PR#874 with new-creation-form.
      • monkey
        Shubh: I've changed the target branch for #876 to new-creation-from, but that created some merge conflicts to resolve
      • Shubh
        i was thinking of first merging it with master then with new-creation-form branch
      • monkey
        Ah, sorry I didn't understand. Let me revert the target branch change
      • OK, 876 merged
      • So now I get the same conflicts when updating new-creation-from with latest master. Let me see if I can resolve them easily
      • Shubh
        let me know if it is okay if i will merge master with new-creation-form branch
      • monkey
        Oh, if you want to do it go right ahead !
      • Conflicts don't seem tooooo bad
      • Shubh
        monkey: Pushing direct commit to new-creation-form (main repo) would be ok, right?
      • monkey
        Is it just the merged commit?
      • If so no problem
      • If it's new code It's best to open a PR so that there's a paper trail for reviews etc.
      • Shubh
        merge done!
      • monkey
        Neat! Now one final merge conflict to resolve on 874 and I will deploy it on the test server
      • Shubh
        done!
      • Ah wait need some modifications
      • monkey: now, it is ready!
      • also one minor issue i notice with concurrent-rel is when using manual method to sort items, removed items (`isRemoved:true`) still shows up in series editor. though fixed it in this branch.
      • monkey
        Good catch
      • Shubh: This doesn't look right, was it lost in a conflict resolution? https://github.com/metabrainz/bookbrainz-site/p...
      • Shubh
        I don't think we need this anymore since it only add existing entities to relationship section which no longer require
      • rels*
      • that's why i was so keen on merging the concurrent PR before this
      • monkey
        I'll have to check, but I think we need this code. This is for mergin entities, for example merging author A into author B, all relationship between author A and other entities should be recreated with author B instead of author A
      • Shubh
        Ah my bad , another look and i deleted the wrong code. will fix it
      • monkey
        👍
      • elgranRoble joined the channel
      • BenOckmore joined the channel
      • BenOckmore
        monkey: was doing some coding on -site and -data last week, and was thinking it's quite a hassle to clone them separately and keep things in sync when implementing new stuff in -data. So, I've made a test repository which merges the two repos as if -data had always been a part of site... Test repository is here: https://github.com/LordSputnik/bookbrainz and the merge is here:
      • https://github.com/LordSputnik/bookbrainz/commi... . It seems to run fine with my (slightly old) database clone, and there doesn't seem to be a drop in test pass rate