Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pullrequests ( Opened in 2016 with updates to February 2022 / Merged into master on 7th February 2022 ) - or what is the integration merge workflow? #280

Open
ifrh opened this issue Aug 20, 2017 · 24 comments
Open

Pullrequests ( Opened in 2016 with updates to February 2022 / Merged into master on 7th February 2022 ) - or what is the integration merge workflow? #280

ifrh opened this issue Aug 20, 2017 · 24 comments

Comments

@ifrh
Copy link
Contributor

ifrh commented Aug 20, 2017 •
edited
Loading

Hi there,

In my pullrequest #238 from 27 Jul 2016 @mhecker and I discussed #238 (comment) about handling that big pullrequest 6 Dec 2016 (https://github.com/KITPraktomatTeam/Praktomat/pull/238/files/0e6569da20e819e1ccf80da7e45d28556aaab319) - which would change +1,614 −147 lines of code. In consequence I splitt my changes into multiple pull requests (february and march 2017). After splitting up into multiple pullrequests I closed that single big pull request.

I would be very happy, if for someone our additional features or other changes are interessting.

contentlist

  1. make .gitmodules http_proxy or https_proxy compatible again (our server is behind a http_proxy https_proxy) (the new pull request is closes Feature-Request: #260  #261 )
  2. bugfix for bug Praktomat Timeout didn't work with every kind of checkers. <== Includes Patch for Praktomat/src/utilities/safeexec.py #255 (in Praktomat/src/utilities/safeexec.py) (identical to my reverted commit f4f86ff ) (the new pull request is This fixes #255 Praktomat/src/utilities/safeexec.py #262 )
  3. reintroducing a "DiffChecker" again (Yes, we have the need for it) (the new pull request is Feature DiffChecker (reintroduce) #263 )
  4. Our "AutoAttestChecker" (the new pull request is Feature AutoAttestChecker : limitation a task solution can only … #264 )
  5. introducing missing admin actions to avoid working directly from dbshell:
    i.e. after changing Task-Checkers while upload-time is running
    UseCase: "JUnit-Test (and trainer solution) was wrong but studentuploads really solved the task"
    UseCase: "Task-Conditions changed during lesson or lecture"
    (the new pull request is Task-Admin-Feature: additional actions avoid working via dbshell #265 )
  6. bugfix for bug Resubmit Solution => Java Compiler failed <= includes patch for src/solution/models.py #232 (Solution resubmit bug) (the new pull request is Bugfix for issue #232 (resubmit bug) #266 )
  7. Modification for Trainer and Tutor in Attestation View so that they could see Solutionfiles if there exists no annotated solutionfiles (the new pull request is Attestation_view-Template: Allow Tutor+Trainer to watch unannotated ... #267 )
  8. Bugfix for typos in src/settings/*.py (the new pull request is Bugfix small typos in src/settings/* #268 )
  9. Extension in TaskModel for configuring "Dynamic waiting time" and "number of max uploads" (the new pull request is Feature: uploaders dynamic waiting time and max upload limit #269 )
  10. Sorting Tasklist using Task Title and Submission date (the new pull request is Feature: sort tasklist via task title and not only using submission date #270 )
  11. Additional button for uploading model solution to avoid scrolling (the new pull request is TaskAdmin : Add upper site button for adding model solution #271 )
  12. "Praktomat - LDAP connection" (with dummy values for showing config) (the new pull request is Feature LDAP binding , letting switch between Shibboleth and LDAP via setting variables #272 )

And additional I created two more pullrequests:

Best regards,
Robert
@physikerwelt
Copy link
Contributor

@ifrh @mhecker the new semester is approaching... I'm particularly interested in #264. Please let me know of the plans for dealing with pull requests. For me, it's not a big effort to work on an extra fork. However, that's not ideal, since this might lead to divergence of branches used in different universities.

@physikerwelt
Copy link
Contributor

any updates here? @schlom @KITPraktomatTeam @oberam-eng

@ifrh
Copy link
Contributor Author

ifrh commented Oct 31, 2017

@physikerwelt : Well, I cannot merge my own pull requests into master of this repository. So I am waiting, too.

@ifrh ifrh changed the title Open pullrequests Open pullrequests (in respect to new Branch for Django 1.11 compatibility [Headline canged at 2018-01-29]) Jan 29, 2018
@ifrh ifrh changed the title Open pullrequests (in respect to new Branch for Django 1.11 compatibility [Headline canged at 2018-01-29]) Open pullrequests (in respect to new Branch for Django 1.11 compatibility [Headline changed at 2018-01-29]) Jan 29, 2018
@ifrh
Copy link
Contributor Author

ifrh commented Jan 29, 2018 •
edited
Loading

@ratefuchs
Hi there, I noticed that there is a new branch with name "upgrade_django_1_11" containing commit 750fdcf ("fix more python3 related problems"). Is planned while upgrading Praktomat to Django 1.11 and Python 3 to include any of above mentioned pull requests?

Or should we organize between two semesters in summer 2018 a "praktomat update and merge programming workshop"?

@ifrh ifrh changed the title Open pullrequests (in respect to new Branch for Django 1.11 compatibility [Headline changed at 2018-01-29]) Open pullrequests (in respect to new Branch for Django 1.11 compatibility) Feb 5, 2018
@ifrh ifrh changed the title Open pullrequests (in respect to new Branch for Django 1.11 compatibility) Open pullrequests - or what is the intregration merge workflow? Jul 10, 2018
@ifrh ifrh changed the title Open pullrequests - or what is the intregration merge workflow? Open pullrequests - or what is the integration merge workflow? Jul 10, 2018
@ifrh
Copy link
Contributor Author

ifrh commented Jul 10, 2018

I aksed in January if I should organize a programming meeting for you and us located at my place of employment. There was no answer - so I decided to take holidays.
How likley is the situation, that some of above pullrequests are going to become merge into master?

@physikerwelt
Copy link
Contributor

@ifrh I am also very interested in collaborating in an actively maintained tool such as praktomat. While I like praktomat my main wish is to end up with an infrastructures that everybody (foremost all CS course instructors in Germany) can (not only use but also) contribute to.

I will continue promoting this idea and try to organize a movement/alliance/interest group to make this happen.
link

@ifrh
Copy link
Contributor Author

ifrh commented Dec 15, 2018

I updated my branches and pullrequests
you can take all our features and bugfixes with one pullrequest:
#300

Or you can take each feature or bugfix seperate:

#299 (using django 1.11.15)
#298 (additional checkers for CUnit, CppUnit, C/C++ Compiler, C Linker)
#297 (changes in CheckstyleChecker to become compatible with checkstyle Version 8.14)
[bug in Checkstyle found by a Praktomat maintainer had been closed :
https://github.com/checkstyle/checkstyle/issues/5127 ]
#296 (Fix a migration file for isabelle checker)
#295 (changes in readme)
#293 (changes in requirements)
#290 (changes in readme)

The following pull requests are my known ones (all are updated with merging current KITPraktomatTeam master branche). Describtion see obove in #280 (comment) ) :
#274
#273
#272
#271
#270
#269
#268
#267
#266
#265
#264
#263
#262
#261

@ifrh
Copy link
Contributor Author

ifrh commented Dec 15, 2018

@mhecker , @ratefuchs , @KITPraktomatTeam , @nomeata : How likely are the acceptance and the merge of our features / bug fixes in master?

@ifrh
Copy link
Contributor Author

ifrh commented Jan 23, 2019

Is any answer possible?

@ifrh ifrh changed the title Open pullrequests - or what is the integration merge workflow? Open pullrequests ( with updates in Dec 2018) - or what is the integration merge workflow? Jan 23, 2019
@nomeata
Copy link
Contributor

nomeata commented Jan 23, 2019

I have left KIT since then, and can no longer answer that question. But from the side lines I observe that there is simply more interest in this work outside KIT, so I suggest @ifrh and @physikerwelt should just go ahead and maintain a fork of Praktomat in a different Github repository, maybe pulling from this one if you don’t want it to diverge. That's the point of git, after all! Maybe at some point there will be more collaboration again.

@physikerwelt
Copy link
Contributor

@nomeata I think that's the best thing we can do at the moment. However, maintaining a fork is a lot of effort. At least from my experience with https://github.com/wikimedia/MathJax

Moreover, forking is more a short-time solution, especially given the situation that research and teaching assistants usually have non-permanent positions.

@ifrh ifrh changed the title Open pullrequests ( with updates in Dec 2018) - or what is the integration merge workflow? Open pullrequests ( with updates in Dec 2018 and March 2020) - or what is the integration merge workflow? Mar 16, 2020
@ifrh
Copy link
Contributor Author

ifrh commented Mar 16, 2020

Hi there, I updated (March 2020) all my single fix and feature branches: available as seperated pull requests.
And I merged them together into the branch hbrs_public, available as single pullrequest #300

Hopefully some of that features can get merged or cherry picked into the current master.

@ifrh ifrh changed the title Open pullrequests ( with updates in Dec 2018 and March 2020) - or what is the integration merge workflow? Open pullrequests ( with updates in Dec 2018, March and July 2020) - or what is the integration merge workflow? Sep 5, 2020
@ifrh ifrh changed the title Open pullrequests ( with updates in Dec 2018, March and July 2020) - or what is the integration merge workflow? Open pullrequests ( with updates in Dec 2018, March to July 2020 and October 2020) - or what is the integration merge workflow? Oct 31, 2020
@ifrh
Copy link
Contributor Author

ifrh commented Apr 10, 2021 •
edited
Loading

@mhecker : Lets have a look to H-BRS pullrequests, again:
All open H-BRS pullrequests were updated in October 2020 via merging KITPraktomatTeam:master into them.
I just now see, that KITPraktomatTeam:master had been updated in November 2020.
I'll merge that KITPraktomatTeam:master state into all open H-BRS pullrequests.

@physikerwelt
Copy link
Contributor

@ifrh I am going to use Praktomat at Uni Wuppertal in the winter term. Do you have a branch that contains all your improvements as I guess there is little hope that merges to master will happen shortly.

@ifrh
Copy link
Contributor Author

ifrh commented Nov 5, 2021

Just for the record. @physikerwelt and I had a nice telephone call. In winter term 2021 I am reusing the Praktomat sources from winter term 2020 with Python 2: That is commit 0f8f93e in branch hbrs_public of fork ifrh/Praktomat.

Since April 2021 I had no time to update all my branches with open pullrequests. But as I wrote in April #280 (comment)

I'll merge that KITPraktomatTeam:master state into all open H-BRS pullrequests.

as soon as I have time and combine them again in my branch hbrs_public.

@physikerwelt
Copy link
Contributor

One follow up question. Will you be able to update your systems to python3 soon. January 1, 2020 was EOL for python2 so I think it might be a good idea to upgrade.

@ifrh
Copy link
Contributor Author

ifrh commented Nov 5, 2021 •
edited
Loading

At time of writing this comment KITPraktomatTeam/Praktomat:master has its latest commit 7642e06 (19 Apr 2021). This state of KITPraktomatTeam/Praktomat:master is merged into ifrh/Praktomat:master32 , which is my Python 2 and Python 3 compatible branch without our hbrs feature... it is more or less like master branch but with Python 2 and Python 3 support.
After bugfixing ScriptChecker via commit 06affd9 and commit e2474db (both 27 June 2021) inside ifrh/Praktomat:master32,
this branch is now compatible with Python 2 and multiple Python 3 versions as far as it can be determined via calling
./src/manage-test.py test accounts attestation checker configuration solutions tasks hbrs_tests

Travis results for

  • Python 2.7
  • Python 3.5, 3.6, 3.7, 3.8, 3.9

you can see here: https://app.travis-ci.com/github/ifrh/Praktomat/builds/230969219

@physikerwelt To answer your question:
My merging route would be

  1. merge ifrh/Praktomat:master32 into all of my H-BRS branches with corresponding open pullrequests ( https://github.com/KITPraktomatTeam/Praktomat/pulls/ifrh ) .
  2. try to combine all those branches with an octopus merge into the branch ifrh/Praktomat:hbrs_public as an update for pull request Hbrs public 2018 (updated March and July 2020) #300

@physikerwelt
Copy link
Contributor

@ifrh I looked at the open pull requests. For our instance at uni Wuppertal I do not want to include the python2 compatible layer: Python2 is dead and I think it should rest in peace;-) Honestly, adding python2 compatible will neither improve performance nor security nor readability / maintainability of the code. However, most of your pull request depend on adding Python2 compatibility. This makes them hard to review and rebase. Therefore I am trying to find out if you really need the Python2 compatibility of if it would be easier if you updated your systems to python3.

@ifrh
Copy link
Contributor Author

ifrh commented Nov 14, 2021

@physikerwelt Thanks for looking at my open pull requests. Now I think you are right. Since KIT master branch is Py3 only, I should rewrite my bug fixes and features to be python 3 only, too; so that reviewing, cherry picking or merging my branches can be done more easily for others who do not want to get python 2 stuff back.
So at least all my bugfix and feature branches will be python 3 without python 2 "compatibility layer".
The branch hbrs_public will contain master and all of my bugfixes and features all as python 3, so that it'll be possible to get all our changes at once.
But at least two branches will contain a python 2 "compatibility layer" as long as I have a need for them:
That branches are master32, which is like master but with py2,py3 support, and a new branch called hbrs_public32, which contain like hbrs_public all of my bugfixes and features but stay compatible with py2 and py3.

Unfortunately I cannot give any information on the time horizont.
@ratefuchs , @mhecker I will soon list here which of my feature branches have lost their Python 2 layer as soon as I have finished switching them to python 3.

@ifrh
Copy link
Contributor Author

ifrh commented Jan 22, 2022

unmerged "python 3 only" pull request #317 fixing issue #316 ("prevent importing the same final grade rating scale and rating scale items while importing tasks not as template") containing commits

passed travis run https://github.com/ifrh/Praktomat/runs/4907759033

@ifrh ifrh changed the title Open pullrequests ( with updates in Dec 2018, March to July 2020 and October 2020) - or what is the integration merge workflow? Pullrequests ( Opend in 2016 with updates to February 2022 / Merged into master on 7th February 2022 ) - or what is the integration merge workflow? Feb 7, 2022
@ifrh
Copy link
Contributor Author

ifrh commented Feb 7, 2022

@ratefuchs thank you for your merge marathon from today (7th February 2022) 👍

I had update my branch ifrh/Praktomat/master to become equal with
current KITPraktomatTeam/Praktomat/master at commit e55bf25 .

At the end of todays merge marathon the master branch at commit e55bf25 did sadly not pass the unit test run on travis:

https://app.travis-ci.com/github/ifrh/Praktomat/builds/246035409


Praktomat/wsgi/praktomat.wsgi", line 3, in <module>

    import mod_wsgi

ModuleNotFoundError: No module named 'mod_wsgi'

the fix should be adding mod_wsgi into requirements.txt

The reason for not passing the unit test is:
The unit test code for praktomat.wsgi of commit ifrh@600e270 inside pullrequest #268
didn't "know" changes inside praktomat.wsgi of commit physikerwelt@3c697c0 inside pullrequest #279

@ifrh
Copy link
Contributor Author

ifrh commented Feb 8, 2022 •
edited
Loading

@ratefuchs thank you for your merge marathon from today (7th February 2022) 👍
[...]

Praktomat/wsgi/praktomat.wsgi", line 3, in <module>

    import mod_wsgi

ModuleNotFoundError: No module named 'mod_wsgi'

the fix should be adding mod_wsgi into requirements.txt

The reason for not passing the unit test is: The unit test code for praktomat.wsgi of commit ifrh@600e270 inside pullrequest #268 didn't "know" changes inside praktomat.wsgi of commit physikerwelt@3c697c0 inside pullrequest #279

Yes, a change in requirements.txt an addition in .travis.yml and the handling of mod_wsgi.process_group inside wsgi/praktomat.wsgi had to be changed.

Fixed in pull request #318

@ifrh ifrh mentioned this issue Feb 8, 2022
Merged
@ifrh
Copy link
Contributor Author

ifrh commented Feb 19, 2022 •
edited
Loading

@ratefuchs some more fixed "merge bugs" in pull request #320

@ifrh
Copy link
Contributor Author

ifrh commented Mar 9, 2022

@ratefuchs some more "merge bugs" described in issue #324

Thank your for your merge marathon !

@ifrh ifrh changed the title Pullrequests ( Opend in 2016 with updates to February 2022 / Merged into master on 7th February 2022 ) - or what is the integration merge workflow? Pullrequests ( Opened in 2016 with updates to February 2022 / Merged into master on 7th February 2022 ) - or what is the integration merge workflow? May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nomeata @physikerwelt @ifrh