Jump to content
dummzeuch

Creating a "pull request" for jvcl

Recommended Posts

I have just discovered a bug in TJvCustomAppIniStorage.DeleteSubTreeInt and fixed it. So now I would like to contribute it back to the JVCL.

 

Just so I am sure I understand what I need to do to create a pull request for a bugfix in the JVCL:

 

  1. Create a GitHub account (done)
  2. Create a repository on GitHub
  3. Clone the existing JVCL repository to a local directory using git
  4. Change the parent repository to my repository somehow on the command line
  5. Make the change
  6. Commit and push it to my repository
  7. Somehow (I am not quite sure how yet), create a pull request on that using the github website.

 

And all that so somebody can actually look at it and decide that it is a bug and copy my code to fix it?

 

Is it really that complicated or am I missing something here?

Share this post


Link to post

Thomas,

You shouldn't need to create a repository, just clone the JVCL to your machine.

If you use the GitHub Desktop application, it allows you to submit your changes as a Pull request. Not sure how this is achieved from the command-line but I'll have a look.

  • Like 1
  • Thanks 1

Share this post


Link to post

Looks like you cannot do it from the command line with the out of the box stuff.

  • Thanks 1

Share this post


Link to post

OK, I'll give the GitHub Desktop app another try. Thanks a lot!

 

btw:

unit JVJclUtils;

function LeftStr(const S: string; N: Integer): string;
{ LeftStr return a string right-padded to length N with blanks. }

WTF?

Took me quite a while to figure out why my bugfix didn't work.

Share this post


Link to post

Sorry to be such a noob:

I have cloned the repository to a local directory, made the change and committed it.

Pushing it of course won't work because I have no write access the the remote repository.

 

If I select "Branch -> Create Pull Request" the app asks me to push the commit (which I don't because see above) and the  takes me to the github page. I'm lost now.

Are you sure I don't need to clone the repository to my own github account first?

Share this post


Link to post

You need to commit the change to the local repository and then "pull" the request.

Share this post


Link to post

OK, I hereby admit (yet again) that I am apparently too stupid to work with git.

 

Thanks for trying to help, but I won't bother. I have more important things to do.

Share this post


Link to post

Forget about it. The concept of pull requests eludes me as well. And I consider myself a regular git user.

  • Like 1

Share this post


Link to post
Posted (edited)

Guys, while pulling updates from a remote repo is part of git (e.g. git pull from the command line for example),  a "pull request" is not something intrinsically part of git, but something that github institutes, or that we agree between ourselves as developers.  It literally is saying to someone else "Please {git} pull from me as I've got something you might want to review/use."  

 

It follows that in order for someone else to "pull" changes from you, that you must have some accessible place for them to pull from.

 

Obviously users on the internet won't have access to your local repo on your developer PC, so they cannot pull from there.  Nor would you normally be able to push to their repo (or their github repo) if that is where you originally pulled from (unless you're one of the project administrators/owners of course.)

 

There therefore needs to some other repo, that must belong to you and is accessible so that someone else can pull from, that you can also first push to.  (Just like there needs to be a shared SVN server, say, if you want another dev to get some changes you've made and are using subversion.)

 

And this is where github comes in.  It provides an easy to access place where you can fork and create your own remote repositories (from others) that are therefore just as easily accessible by others too.  

 

All that background is to help you understand the following:

 

The normal process for working on github, if you want to contribute changes, is to 

1.) Fork the repo of the project you want to contribute to in your github account. This creates a cheap remote repo (think "my own SVN server at github" if that helps) that belongs to you, than you can push to.

2.) Clone this fork (your own copy of this github repo) to your local PC.

3.) Do your work, commit locally.  Once totally happy, git push, which obviously goes to your own remote repo (from the fork.)

4.) Now you're a few revisions ahead of the original repo, and you can then tell the original upstream repo maintainers that you'd like them to pull from your repo as you've got changes to fix some issue or whatever.  In github, you do this by simply clicking "New Pull request" button in github.

image.thumb.png.570295651ea8779686aa31f224812093.png

 

Hope that helps!

 

Edit: By the by, if you've previously pulled from someone else's project directly, made some changes and now want to push this somewhere else, it's quite easy to fork the project "after the fact" and then tell git/update that "remote"/"upstream" is now else, to e.g "push" to your fork instead. (It's also possible to have multiple remotes if you want, but I digress...)

Edited by ByteJuggler
  • Like 4
  • Thanks 8

Share this post


Link to post
1 hour ago, dummzeuch said:

Are you sure I don't need to clone the repository to my own github account first?

Just to emphasise: Yes -- they must have somewhere to pull from that's accessible, which is kind of the point of github  (So you're not too stupid to use git after all! :classic_cheerleader:)

Share this post


Link to post
Posted (edited)
1 hour ago, David Hoyle said:

You need to commit the change to the local repository and then "pull" the request.

Without wanting to be polemical and for the sake of anyone else reading this thread: As written that doesn't make sense, I assume you must've meant to write: "You need to commit the change to the local repository, then push it to your remote fork, then create a "pull" request in github" against/from your remote fork.  

Edited by ByteJuggler

Share this post


Link to post
1 hour ago, dummzeuch said:

I am apparently too stupid to work with git

"GIT - the stupid content tracker"

 

In GitHub click 'Fork' on the jvcl repo, use that repo for your local feed.
Push your changes to that repo, then use GitHub 'Pull Request'.

 

In the end it works out to be a better system, you have a copy of the code you are using in GitHub, regardless of jvcl integrating it or not.
When jvcl is updated you can use GitHub to see any changes before deciding to update your Fork.

 

One of the recent 'TortoiseGit' versions has a create Pull Request URL now, but I haven't used it.

 

Share this post


Link to post

OK, so what I originally wrote was basically correct:

  1. Fork the original repository to create your own copy of it on GitHub
  2. Create a local clone of your own fork using e.g. GitHub Desktop
  3. Make the change.
  4. Commit it
  5. Push it to GitHub
  6. On the repository's website create a pull request.

I just did that, let's see what happens.

https://github.com/project-jedi/jvcl/pull/74

 

Thanks everybody who helped. The crucial step is to first *fork* the original repository on GitHub rather than clone it.

 

(OK, maybe I am not to stupid to use git after all.)

 

 

  • Like 2

Share this post


Link to post

Nice.  Now nosy people like myself can also go have an, er, nosy around to see what you've done and even grab the change if I want it. 🙂 

Share this post


Link to post

Hm, my pull request is still open, but at least it has gotten some comments. JVCL seems rather inactive nowadays.

 

Now, I would like to add another pull request for the same file but an unrelated change. Again I seem to be too stupid to do that. Every time I try it I end up with the existing pull request.

 

Share this post


Link to post
Posted (edited)

How to work with git pull requests

Quote

Pull requests

Good pull requests, patches, improvements and new features are a fantastic help. They should remain focused in scope and avoid containing unrelated commits.

Please ask first before embarking on any significant pull request (e.g. implementing features, refactoring code, porting to a different language), otherwise you risk spending a lot of time working on something that the project's developers might not want to merge into the project.

Please adhere to the coding guidelines used throughout the project (indentation, accurate comments, etc.) and any other requirements (such as test coverage).

Adhering to the following process is the best way to get your work included in the project:

  1. Fork the project, clone your fork, and configure the remotes:

    
    # Clone your fork of the repo into the current directory
    git clone https://github.com/<your-username>/<this-repro-name>.git
    # Navigate to the newly cloned directory
    cd <folder-name>
    # Assign the original repo to a remote called "upstream"
    git remote add upstream https://github.com/<your-username>/<this-repro-name>.git

     

  2. If you cloned a while ago, get the latest changes from upstream: 

    
    git checkout master
    git pull upstream master

     

  3. Create a new topic branch (off the main project development branch) to contain your feature, change, or fix: 

    
    git checkout -b <topic-branch-name>

     

  4. Commit your changes in logical chunks. Please adhere to these git commit message guidelines or your code is unlikely be merged into the main project. Use Git's interactive rebase feature to tidy up your commits before making them public. Also, prepend name of the feature to the commit message. For instance: "SCSS: Fixes compiler results for IFileListener.\nFixes #123"

  5. Locally merge (or rebase) the upstream development branch into your topic branch: 

    
    git pull [--rebase] upstream master

     

  6. Push your topic branch up to your fork: 

    
    git push origin <topic-branch-name>

     

  7. Open a Pull Request with a clear title and description against the master branch.

Rule of thumb:

 

For each pull request you need a branch.

Edited by Schokohase

Share this post


Link to post
58 minutes ago, Schokohase said:

For each pull request you need a branch.

I can imagine that the excessive use of branches in DVCSs may frighten users grown up with SVN. I also needed some time to get familiar with that in the beginning, but it makes life a lot easier after I was able to grasp the concept.

  • Like 1

Share this post


Link to post
Posted (edited)

Or in other words: In order to create a pull request, you must first become a master of git.

 

It's really annoying that it is that complicated. A pull request basically is a patch, with a comment. Something that would be possible to create with a text editor (at least for the simple cases I am talking about).

 

Today I found lots of issues which I fixed with my local copy of jvcl / jcl years ago. I am sure I filed bug reports including patches back then. Aparently nobody ever looked at them, because they are still there in the current version.

 

Examples:

* Missing raise for a ESomeException.Create

* missing Create in raise ESomeException(SomeString)

 

I get the impression that "Project Jedi" nowadays is down to a single person or maybe two (I see commits by obones and ahausladen) who are simply swamped with requests.

 

And that's not just Jedi, there is also dxgettext which is unmaintained for all practical purposes (I have been the only contributor for years and that's only because I still have got write access to the repository, which is more of an accident than planned, and I only sometimes make changes to it because that's the tool we use at work). Other projects are obviously unmaintained.

 

My impression is that there may still be many Delphi developers, maybe their number is even raising, but the "Delphi Community" of 20 years ago no longer exists.

 

</rant>

Edited by dummzeuch
  • Like 2

Share this post


Link to post
12 minutes ago, dummzeuch said:

A pull request basically is a patch, with a comment.

Indeed, and you can as well send that patch via email to the person supposed to take care of it. On the other hand, if you want to use a Pull Request, so any maintainer of the original repo can take care of it, you must somehow provide that patch in a form that it actually can be pulled - i.e. in form of a forked clone containing the patch. 

 

The lack of active maintainers for these projects (as unfortunate as it is) cannot be blamed on the DVCS, though.

Share this post


Link to post
9 minutes ago, Uwe Raabe said:

Indeed, and you can as well send that patch via email to the person supposed to take care of it. On the other hand, if you want to use a Pull Request

That's just the thing: I don't really want to use a pull request, I'd rather send a patch. But I was told by one of the maintainers that he won't accept a patch but only pull requests.

So I tried, and after realizing how much work that involves for even a one liner, I will probably no longer bother and phase out or at least reduce the use of the Jedi code at work.

Share this post


Link to post
2 hours ago, Schokohase said:

For each pull request you need a branch

 

Didn't know that, here is a way to do that within GitHub

Share this post


Link to post
36 minutes ago, dummzeuch said:

It's really annoying that it is that complicated

 

There is a benefit in maintaining your own fork, wish it was simpler to find the most up to date fork though and fork that 🙂

 

Also generating a branch before a making a pull request isn't a big deal as long as one knows about that..

 

  • Like 1

Share this post


Link to post
Posted (edited)
1 hour ago, TiGü said:

But all effort is pointless if none of the maintainers integrates the pull requests.

 

https://github.com/project-jedi/jvcl/pull/56

It is also pointless to place a pull request if you know that it will break code for Versions < XE3 while the library clearly states

Quote

It supports Delphi/C++Builder 6 and newer.

 

The contributer knows it, but did not provide a solution in code

Quote

Maybe the string helper methods (introduced in XE3) are not suitable for older versions of Delphi.

In that case, please replace them with the usual old school string handling technique (Pos(), Delete()...).

 

Edited by Schokohase

Share this post


Link to post
Posted (edited)
49 minutes ago, Schokohase said:

It is also pointless to place a pull request if you know that it will break code for Versions < XE3 while the library clearly states

 

The contributer knows it, but did not provide a solution in code

 

Especially with new contributors to open source (but also in general) it's usually advisable in the interests of friendly cooperation to exercise some patience and politeness and give contributors some friendly feedback about their pull requests if they should accidentally miss this or any other type of requirement that change requests should comply with (and to do so in the context where the pull request was submitted.)  If the JCL/JVCL maintainers are not giving at least this feedback to people trying to contribute then that's rather a shame.  😞

Edited by ByteJuggler
  • Like 2
  • Thanks 1

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×