refactor: refactored platform assignment into get_platform function #29971

pull iw4p wants to merge 1 commits into bitcoin:master from iw4p:fix/platform-consistency changing 1 files +18 −15
  1. iw4p commented at 12:25 pm on April 26, 2024: none
    Improved code readability and maintainability by moving platform identifiers into a dictionary.
  2. fix: refactored platform assignment into get_platform function dbe44b5990
  3. DrahtBot commented at 12:25 pm on April 26, 2024: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    No conflicts as of last run.

  4. DrahtBot added the label Refactoring on Apr 26, 2024
  5. in test/get_previous_releases.py:267 in dbe44b5990
    262+    for pattern, target in platforms.items():
    263+        if fnmatch(host, pattern):
    264+            return target
    265+
    266+    print(f'Not sure which binary to download for {host}')
    267+    return None
    


    maflcko commented at 12:31 pm on April 26, 2024:
    Not sure. Now there are two functions and more lines of code, so this is harder to read and to maintain
  6. laanwj commented at 12:46 pm on April 26, 2024: member
    i understand you’re only trying to help, but please don’t flood the project with small Python script refactorings. We’re severely bottlenecked on review, and tend to have a philosophy to not fix what isn’t broken.
  7. iw4p commented at 12:51 pm on April 26, 2024: none
    I understood but if the philosophy is to not fix what isn’t broken, so what “refactor” (for structural changes that do not change behavior) means? I am really trying to start reading the code from easy parts to contribute and refactor codes and update them. Some codes are for more than 4 years ago which are using subprocess for a single curl refrence which is not optimized when python’s providing requests built-in library.
  8. laanwj commented at 1:00 pm on April 26, 2024: member

    Sure, but for example: if you’re going to do things like optimization, consider if speed is important in the process at that point (in many cases it’s not, that’s why the tool is in Python and not C++). If it is important, we’ll also need to see benchmarks that the speed is noticibly faster.

    If you’ve been involved for a while and know where the pain points are, this is easier to know. For new contributors I wouldn’t really recommend doing refactors at all.

    If you’re looking for something to work on (which is great!), please check the https://github.com/bitcoin/bitcoin/labels/good%20first%20issue label.

  9. iw4p commented at 1:08 pm on April 26, 2024: none
    I appreciate your response and explanation. I’ll take a quick look into it!
  10. maflcko commented at 1:19 pm on April 26, 2024: member

    Also:

    Getting started to contribute to Bitcoin Core

    Setting up your development environment

    New developers are very welcome and needed. There are a lot of open issues of any difficulty waiting to be fixed. However, before you start contributing, familiarize yourself with the Bitcoin Core build system and tests. Refer to the documentation in the repository on how to build Bitcoin Core and how to run the unit and functional tests. Once that is done, you are all set.

    If you need more help getting started, please refer to the following resources:

    Pick something to work on

    If you are looking for useful contributions to help out with, you can

    • Spend time on in-depth review of open pull requests and compare your review with the review done by others. This may be time-consuming, but it will teach you relevant skills for this project and for yourself to apply elsewhere. Moreover, on some pull requests there remain small follow-ups that were found during review, so you’ll naturally find stuff to work on if you start out by reviewing pull requests done by others.
    • Search through the good first issues or the ones that are up for grabs. Some of them might no longer be applicable. So if you are interested, but unsure, you might want to leave a comment on the issue first.
    • Write tests to improve the coverage. Any kind of test is welcome and coverage information can be obtained from a relatively recent coverage report. If you are unsure, don’t hesitate to check back first.
    • Help with review and testing. There are easy ones such as the gui and rpc. However, review on any open pull request is welcome. Review will also help you understand the codebase better.
    • Help on meta projects related to Bitcoin Core, such as https://github.com/corecheck.
    • Join the Bitcoin Core IRC channel(s) and leave a comment to share what you are interested in.
  11. fanquake commented at 3:32 pm on May 31, 2024: member
    Thanks for the contribution, however we are going to leave this code as-is for now.
  12. fanquake closed this on May 31, 2024


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-28 22:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me