Netgate Discussion Forum
    • Categories
    • Recent
    • Tags
    • Popular
    • Users
    • Search
    • Register
    • Login

    Submitting PRs for packages in the pfSense/FreeBSD-ports repo?

    Scheduled Pinned Locked Moved Development
    23 Posts 7 Posters 2.4k Views
    Loading More Posts
    • Oldest to Newest
    • Newest to Oldest
    • Most Votes
    Reply
    • Reply as topic
    Log in to reply
    This topic has been deleted. Only users with topic management privileges can see it.
    • jimpJ
      jimp Rebel Alliance Developer Netgate
      last edited by

      @bmeeks:

      Another helpful tip that benefits the pfSense guys during code review is to break your package changes into a series of commits instead of one giant commit.

      While that is nice for review, it makes it more difficult for us to cherry-pick that change to other branches, such as copying the commit(s) from devel to RELENG_2_4_0, RELENG_2_3, and RELENG_2_3_4.

      It's nice to have them all squashed into a single commit before it's merged, but we have some scripts that help us deal with that so it's not the end of the world.

      Remember: Upvote with the 👍 button for any user/post you find to be helpful, informative, or deserving of recognition!

      Need help fast? Netgate Global Support!

      Do not Chat/PM for help!

      1 Reply Last reply Reply Quote 0
      • luckman212L
        luckman212 LAYER 8
        last edited by

        Thanks jimp, so - single unified commits are preferred for PRs then?

        1 Reply Last reply Reply Quote 0
        • jimpJ
          jimp Rebel Alliance Developer Netgate
          last edited by

          Since we don't merge on github it's a little more complicated, but IIRC you can squash the commits in your PR after review so you can have multiple commits while you work, and then it can be merged as one.

          https://github.com/blog/2141-squash-your-commits
          http://blog.steveklabnik.com/posts/2012-11-08-how-to-squash-commits-in-a-github-pull-request

          Remember: Upvote with the 👍 button for any user/post you find to be helpful, informative, or deserving of recognition!

          Need help fast? Netgate Global Support!

          Do not Chat/PM for help!

          1 Reply Last reply Reply Quote 0
          • D
            doktornotor Banned
            last edited by

            @jimp:

            but IIRC you can squash the commits in your PR after review

            That's what you would need to activate, none of the people submitting PRs can use that. It can only be used on merge. You have kind of an issue with the workflow you are using.

            1 Reply Last reply Reply Quote 0
            • jimpJ
              jimp Rebel Alliance Developer Netgate
              last edited by

              @doktornotor:

              @jimp:

              but IIRC you can squash the commits in your PR after review

              That's what you would need to activate, none of the people submitting PRs can use that. It can only be used on merge. You have kind of an issue with the workflow you are using.

              When I'm working locally, I can squash several commits down to one. I think the same thing can be done in a PR by the submitter, but I'm not 100% on that. I know for certain it can be done before submitting the PR. There are lots of how-tos out there on squashing, such as https://github.com/todotxt/todo.txt-android/wiki/Squash-All-Commits-Related-to-a-Single-Issue-into-a-Single-Commit – Yes, github can do that automatically when doing a merge in their GUI, but that doesn't help us.

              The workflow has to be the way it is because everything has to be sourced from our local gitlab instance. Aside from it being a better flow for us internally, we don't like being stopped dead in our tracks if github goes down, which happens a lot. Some people don't notice how often it happens, but when you're working in git all day, every day, it's more apparent.

              There may also be something we can cook up eventually with local scripts to squash but I'm not sure any of us have looked at it.

              Remember: Upvote with the 👍 button for any user/post you find to be helpful, informative, or deserving of recognition!

              Need help fast? Netgate Global Support!

              Do not Chat/PM for help!

              1 Reply Last reply Reply Quote 0
              • D
                doktornotor Banned
                last edited by

                Well yes you can squash things locally… what you linked regarding GitHub squash feature however is unusable for anyone without merge (and apparently for people with merge privs as well due to the workflow).

                1 Reply Last reply Reply Quote 0
                • jimpJ
                  jimp Rebel Alliance Developer Netgate
                  last edited by

                  Yeah but the question is if you can squash to your own PR after creating the PR, before it gets merged. I'll have to try to test that out.

                  Remember: Upvote with the 👍 button for any user/post you find to be helpful, informative, or deserving of recognition!

                  Need help fast? Netgate Global Support!

                  Do not Chat/PM for help!

                  1 Reply Last reply Reply Quote 0
                  • bmeeksB
                    bmeeks
                    last edited by

                    @jimp:

                    @bmeeks:

                    Another helpful tip that benefits the pfSense guys during code review is to break your package changes into a series of commits instead of one giant commit.

                    While that is nice for review, it makes it more difficult for us to cherry-pick that change to other branches, such as copying the commit(s) from devel to RELENG_2_4_0, RELENG_2_3, and RELENG_2_3_4.

                    It's nice to have them all squashed into a single commit before it's merged, but we have some scripts that help us deal with that so it's not the end of the world.

                    Didn't realize it complicated the cherry picking process.  I'm not a GitHub guru, though.  Based on the conversations that followed in this thread, it looks like there are pros and cons each way (multiple commits versus single giant commit).

                    Bill

                    1 Reply Last reply Reply Quote 0
                    • D
                      doktornotor Banned
                      last edited by

                      Those giant commits definitely suck when searching for regressions. Plus mostly unusable with GitHub web interface, it just won't show anything lots of times, instead complaining about the query taking too much time etc.

                      1 Reply Last reply Reply Quote 0
                      • ?
                        Guest
                        last edited by

                        @jimp:

                        Yeah but the question is if you can squash to your own PR after creating the PR, before it gets merged. I'll have to try to test that out.

                        Yes you can, that's what I do now after initially having a load of commits for typos and additions in my earlier PR's. Now I do a git rebase -i HEAD~x locally, where x is the number of commits to squash and then a git push origin whatever-im-doing –force

                        That syncs it all up nicely. I'm 99.9% certain the PR still points at the branch if I've issued a PR from, that's what the a quick Google of Git says too. My testers end up with one nice patch ID and you end up with a single commit.

                        1 Reply Last reply Reply Quote 0
                        • L
                          loonylion
                          last edited by

                          @bmeeks:

                          …

                          I ran into the exact same problem with the Windows Git client.  It just does not like the FreeBSD-ports repository for some reason (or at least it never worked for me).

                          ...

                          Bill

                          It doesn't work because there's files in the repo with names reserved by Windows (specifically 'LPR' IIRC) I think its probably a holdover from the DOS days but maybe they're still used internally in Windows.

                          1 Reply Last reply Reply Quote 0
                          • ?
                            Guest
                            last edited by

                            You can do all of the pfSense stuff itself in Windows Git Desktop, the FreeBSD ports fails due to the Japanese folder which windows does not like due to some of the folder names.

                            I use a VM FreeBSD with GUI for the FreeBSD ports. Doing the commits on that is a bit more long winded as it all has to be done from a shell.

                            1 Reply Last reply Reply Quote 0
                            • jimpJ
                              jimp Rebel Alliance Developer Netgate
                              last edited by

                              Ideally anything dealing with FreeBSD-ports should happen on a FreeBSD system at some point (either directly or in a secondary capacity) because then you have access to tools like portlint which can check the port structure to ensure it's complete and correct before committing changes, for example it will flag problems with the formatting or content of the Makefile.

                              Remember: Upvote with the 👍 button for any user/post you find to be helpful, informative, or deserving of recognition!

                              Need help fast? Netgate Global Support!

                              Do not Chat/PM for help!

                              1 Reply Last reply Reply Quote 0
                              • First post
                                Last post
                              Copyright 2025 Rubicon Communications LLC (Netgate). All rights reserved.