gesetze-tools
gesetze-tools copied to clipboard
Changes of joacmue
Hey @joacmue
would you be interested in getting your changes in the master? Would that be applicable?
Interested in becoming a maintainer?
Hi @ulfgebhardt, sure, merging that back into your fork would totally work for me. It just seemed pretty dead over the past months ;) I'm not that versed around open source development, so... should I start a PR from my fork to yours?
Not sure what I'm getting myself into by becoming a maintainer, but I'd be open to contribute.
Looking over the other current activities, this one mainly fixes lawdown and adds some image/table features to it (my test case was the StVO which is pretty heavy on images in tables for Traffic Signs and stuff). Might only want to review the changes to lawdown and readme.md and just throw away the other changes. I tried going into the banz_scraper but ran into issues with their page redesign pretty fast. If I recall correctly, I mainly noted down some new candidate links, but stuff seems to be better addressed by #18
Hey @joacmue nice to hear from you. I believe I could incite some activity into the project lately. I was informed that you made some nice changes not to long ago and just wanted to expose those to this repo.
Would you be interested i contributing? I believe we are on a good way to get things running and have an up to date https://github.com/bundestag/gesetze repo soon. With your help sooner and better <3
With the looming Election this year having the repo up to date would save me from millions of questions why its not up to date ;)
So, I just rebased my fork to what happened here but did not get the whole pathlib stuff right away, so most of the changes made there might have gotten lost. Feel free to add them back during the review.
So I just recognized the bigger picture of pushing the actual laws to https://github.com/bundestag/gesetze That puts the work here in a new perspective for me. please stand by while I process this. Kidding aside, once my slightly more involved state stuff in the parser got merged back, I'll see where I feel comfortable helping out on issues. Might need some help with the actual scrapers, though.
I believe we have to copy the PR into the repo or rebase in oder for the tests to run. I cannot accept your suggestions. I think only @joacmue can, since hes the owner of the fork.
@ulfgebhardt We should give @joacmue some time to work on this...
@joacmue Please check which changes are on top of #24 which restored BAnz functionality and adapt your PR accordingly.
I am not sure we can wait @darkdragon-001 since we have so many changes pushed into master. Every commit we make will get us further away from merging this. It is fine with me to just leave this open and cherry pick commits or changes and PR them individual, but then we will not honor the work of @joacmue . If you want to take the effort and see the benefit to get these changes in, please just take over. -> Tho thats just my opinion ;)
Sorry, quite busy at the office right now. I hope to look into rebasing from the original repo to my fork by the weekend. Should be possible to merge after that.
We would love to have you involved @joacmue ! I believe the others have the sam struggle and have to work during the week. I hope that we get another of those sessions like last weekend for the coming one and have the thing in a state where it can do stuff automatically without much human interaction. <3
Yeah, we're in some sort of Deadlock right now. I don't have write access to merge anything here but just invited you as collaborators to my fork. I'll try and merge locally with the current master here to get rid of some of the conflicts currently present.
I honestly don't know how to run the tests, though. Can you point me to some howto? It might actually be worthwhile to make a branch here and bend this PR onto it. I guess we could then run the tests defined here and have less issues with differing writing access...
I think it would be wise to have a rebase on your branch @joacmue . I tried, but its quite complicated and out of my league, since it needs knowledge of what you have done, which parts are important which are not.
Thanks for the hint of rebasing, @ulfgebhardt, I wasn't aware that this is available across forks. As I said, pretty new to this stuff. Please excuse the inconvenience. I did manage to rebase everything, while silently cursing the back-then-Joachim but now everything seems to be up and running again. I still don't quite know how I could start the checks, but I guess that I'm lacking the rights to perform those (given they are defined on the "original" gesetze-tools but need to run over my changed scripts.
So, all conflicts are resolved, the code is rebased to the state of the master and the checks are all passing on my fork. I don't quite know how to get the checks running here, but I guess actually merging this is out of my hands?
@ulfgebhardt @JBBgameich Can someone have a look why the checks aren't running for merge requests from fork? Can I trigger those manually?
@joacmue I added some small changes to your branch. Feel free to have a look and leave a comment if you see things differently. I will have a closer look at lawdown.py
the next days.
Note to myself: When merging this, we should use the squash merge function to maintain a clean git history and squash this many commits in a single patch.
I believe checks are not running because of security reasons. As soon as we agree to merge this, I will just push a branch with your changes and @darkdragon-001 can approve it, we merge it. As soon as you have at least 1 commit in the Orga I will give you member access, which will allow you to push to this repo. So forking will not be needed in the future.
Just tell me when your done & ready. That includes @darkdragon-001 : Would you approve?
@joacmue I added some small changes to your branch. Feel free to have a look and leave a comment if you see things differently. I will have a closer look at
lawdown.py
the next days.Note to myself: When merging this, we should use the squash merge function to maintain a clean git history and squash this many commits in a single patch.
Changes made work for me. Also, squashing will definitely make sanse here :D
As soon as you have at least 1 commit in the Orga I will give you member access, which will allow you to push to this repo. So forking will not be needed in the future.
@ulfgebhardt What do you mean here? Should I be doing something?
Just tell me when your done & ready. That includes @darkdragon-001 : Would you approve?
I will finish the review over the weekend and approve when everything looks fine.
Check ran now since I pushed stuff to a branch in this repo. Please just update the branch and then merge this PR if everything is cool @darkdragon-001 .
See: https://github.com/bundestag/gesetze-tools/pull/38
Depending on how interested @joacmue is to get things done, cherry picking might be a good option here @joacmue I also made a copy of this branch to here: https://github.com/bundestag/gesetze-tools/pull/38 That way the tests do run and it is mergable
@ulfgebhardt I updated the CI to also run on PRs (see #44). When it is merged, I will pull in master to activate those changes. You can close #38 then.
Depending on how interested @joacmue is to get things done, cherry picking might be a good option here @joacmue I also made a copy of this branch to here: #38 That way the tests do run and it is mergable
We could try that, but I've never done that and fear that my development on the fork was too erratic and chaotic to do some reasnable cherrypicking on the basis of commits. I just lacked the discipline of doing only one thingin each commit. I guess I'll have to figure the remaining issues out to merge everything.
Depending on how interested @joacmue is to get things done, cherry picking might be a good option here @joacmue
We could try that, but I've never done that and fear that my development on the fork was too erratic and chaotic to do some reasonable cherrypicking on the basis of commits. I just lacked the discipline of doing only one thingin each commit. I guess I'll have to figure the remaining issues out to merge everything.
I totally agree. I think it is lots easier to just fix the few remaining issues instead of looking over those >50 commits. Most things are working well, just some minor glitches on the way :wink:
You guys are very constructive and do really great work ❤️ @joacmue If interested join our discord channel: https://discord.gg/YcQSvKcKB2
After some more meddling around, all comments should now be adressed. Thanks for the hints on where to clean up my stuff.
@darkdragon-001 what you say? merge?
So, I rebased to the master from here. lawdown.py still works on everything wth the expected results, so I guess I made that right.
However, I noticed that the banz_scraper fails with an error. I copied over the code from master here and I do have the same error, so I guess there's some fault in the code there. Not gonna put cleaning that up in this PR, though ;)