Skip to content

Conversation

rruuaanng
Copy link
Contributor

@rruuaanng rruuaanng commented Dec 2, 2024

In Python, it is recommended to use bool values ​​as conditions, but there are still scenarios where integers are used in pyrepl, and we use while True instead of while 1.
(I hope this is not considered a cosmetic change. They were discovered when I was learning pyrepl.)

@rruuaanng
Copy link
Contributor Author

This change is minor, I don't think it's necessary to add a NEWS and title-label.

@skirpichev
Copy link
Contributor

I hope this is not considered a cosmetic change

I think it is.

@rruuaanng
Copy link
Contributor Author

I hope this is not considered a cosmetic change

I think it is.

But haven't we always advocated not using numbers as a condition?
Maybe this can be used as another solution.

@ZeroIntensity
Copy link
Member

We don't need a NEWS; users won't see or care about this change. With that being said, this is a cosmetic change. I'll let the repl experts decide if they want to take it.

@pablogsal pablogsal merged commit a327810 into python:main Jan 1, 2025
54 checks passed
@pablogsal
Copy link
Member

I decided to land this for consistency but please don't read this as encouraging these kind of changes as normally we tend to reject purely cosmetic changes based on problems for backporting and other considerations

@pablogsal
Copy link
Member

Thanks for your controbution @rruuaanng

@ZeroIntensity
Copy link
Member

Speaking of backporting, do we want this in 3.13?

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot aarch64 RHEL8 LTO 3.x has failed when building commit a327810.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/338/builds/7762) and take a look at the build logs.
  4. Check if the failure is related to this commit (a327810) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/338/builds/7762

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-aarch64.lto/build/Lib/threading.py", line 1054, in _bootstrap_inner
    self.run()
    ~~~~~~~~^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-aarch64.lto/build/Lib/threading.py", line 996, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-aarch64.lto/build/Lib/test/test_interpreters/test_stress.py", line 30, in task
    interp = interpreters.create()
  File "/home/buildbot/buildarea/3.x.cstratak-RHEL8-aarch64.lto/build/Lib/test/support/interpreters/__init__.py", line 76, in create
    id = _interpreters.create(reqrefs=True)
interpreters.InterpreterError: interpreter creation failed
k

@rruuaanng
Copy link
Contributor Author

@pablogsal Thank you for the merge :).

@ZeroIntensity It seems that an exception occurred in the test.test_interpreters.test_stress.StressTests.test_create_many_threaded on aarch64, which is a great start. This should be checked by someone who is skilled.

@ZeroIntensity
Copy link
Member

Don't worry, it's unrelated. I think Eric filed an issue a couple days ago about some intermittent failures on that test.

srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants