Skip to content

rewording of CHANGES.txt info. Changed versionadded to 4.5.0#8

Merged
mwichmann merged 2 commits into
mwichmann:maint/CPPDEFINES-Appendfrom
bdbaddog:maint/CPPDEFINES-Append
Feb 16, 2023
Merged

rewording of CHANGES.txt info. Changed versionadded to 4.5.0#8
mwichmann merged 2 commits into
mwichmann:maint/CPPDEFINES-Appendfrom
bdbaddog:maint/CPPDEFINES-Append

Conversation

@bdbaddog

Copy link
Copy Markdown

See description and:

There will be no version 4.5, these changes will be in version 4.5.0
(There may be a 4.5.1)
Also I think the CHANGES.txt changes are little easier to read and lead with the fact this is all about resolving outstanding issues with CPPDEFINES.

I think it's more useful to list the new test coverage, rather than the files they're changed/added in.

@mwichmann

Copy link
Copy Markdown
Owner

There will be no version 4.5, these changes will be in version 4.5.0. (There may be a 4.5.1)

This is not a hill I'm going to die on, but the version we're making the change in is 4.5; if there are micro versions, we promise not to make API changes in those, so I see no reason to distinguish 4.5.0 specifically.

Comment thread RELEASE.txt Outdated
dictionary keys is now preserved when generating the command line.
Previously these were sorted alphabecially: this may cause a change
which leads to a rebuild.
NOTE: If you use a dictionary to specify your CPPDEFINES, you may see unexpected build results

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"unexpected" seems a bit strong when combined with "results". Unless the possibly changed order affects the actual build, which is really unlikely, it's a single rebuild (and silently, update of sconsign), but the result will be the same.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you update SCons to 4.5.0 and you didn't change any sources, and your project rebuilds. I'd call that unexpected.
This has long been the practice of SCons to warn about such, becuase we almost always will get questions about such if we don't note them.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A rebuild may be unexpected, and this is warning about it. But "unexpected...results" suggests the build may not end up the same, which sounds worse than a rebuild to me. How about this:

NOTE: If you use a dictionary to specify all or part of CPPDEFINES, you may
see one-time rebuilds due to changed order of defines on the command
line. SCons now retains the order of addition of all entries to
CPPDEFINES. In previous versions a dictionary used for addition was
sorted alphabetically before the individual items were added.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to unexpected rebuild ?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unexpected rebuild is more palatable than unexpected result....

@bdbaddog

Copy link
Copy Markdown
Author

There will be no version 4.5, these changes will be in version 4.5.0. (There may be a 4.5.1)

This is not a hill I'm going to die on, but the version we're making the change in is 4.5; if there are micro versions, we promise not to make API changes in those, so I see no reason to distinguish 4.5.0 specifically.

The first release in the 4.5 series will be 4.5.0, and that's where this change is first introduced.
Thus 4.5.0.
Plus as I've discussed before, this is how we've always identified releases, so changing now doesn't really provide value, only possible confusion.

@mwichmann

Copy link
Copy Markdown
Owner

There will be no version 4.5, these changes will be in version 4.5.0. (There may be a 4.5.1)

This is not a hill I'm going to die on, but the version we're making the change in is 4.5; if there are micro versions, we promise not to make API changes in those, so I see no reason to distinguish 4.5.0 specifically.

The first release in the 4.5 series will be 4.5.0, and that's where this change is first introduced. Thus 4.5.0. Plus as I've discussed before, this is how we've always identified releases, so changing now doesn't really provide value, only possible confusion.

There have never been versionadded's in docstrings before I started pushing them so there's no change to existing practice involved. Besides, mimicking what CPython does... and they have a ton of micro releases over the life of a release, unlike us. e.g.

https://docs.python.org/3/library/datetime.html#datetime.date.fromisoformat

@bdbaddog

Copy link
Copy Markdown
Author

Version strings should be consistant across all of SCons, IMHO.

@mwichmann mwichmann merged commit 747e7e0 into mwichmann:maint/CPPDEFINES-Append Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants