Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.4.0 changelog #4210

Merged
merged 26 commits into from
Sep 8, 2021
Merged

2.4.0 changelog #4210

merged 26 commits into from
Sep 8, 2021

Conversation

daschuer
Copy link
Member

No description provided.

@coveralls
Copy link

coveralls commented Aug 16, 2021

Pull Request Test Coverage Report for Build 1142456358

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1402 unchanged lines in 18 files lost coverage.
  • Overall coverage decreased (-0.03%) to 25.966%

Files with Coverage Reduction New Missed Lines %
src/library/basetracktablemodel.cpp 2 21.91%
src/library/browse/browsetablemodel.cpp 2 0%
src/library/trackcollectionmanager.cpp 3 12.46%
src/engine/readaheadmanager.cpp 7 90.83%
src/engine/enginebuffer.cpp 13 86.4%
src/library/proxytrackmodel.cpp 16 0%
src/widget/wlibrarysidebar.cpp 16 0%
src/engine/controls/clockcontrol.cpp 17 71.96%
src/engine/controls/cuecontrol.cpp 21 68.8%
src/skin/qml/qmlplayerproxy.cpp 39 0%
Totals Coverage Status
Change from base Build 1133524699: -0.03%
Covered Lines: 20005
Relevant Lines: 77044

💛 - Coveralls

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
[#4186](https://github.com/mixxxdj/mixxx/pull/4186)

### Controller ###
* ES6 based controller mapping system with module support
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* ES6 based controller mapping system with module support
* ES6 based controller mapping system

Using ES6 modules is still not possible as we're not loading the mappings as a module.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should even explicitly mention that its not supported for now so people don't get confused when they expect it to be supported because ES6 support mandates it.

Copy link
Member Author

Choose a reason for hiding this comment

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

For my understanding #2868 enabled the module support. Is it not yet usable?

Copy link
Member Author

Choose a reason for hiding this comment

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

Anyway, I have improved the sentences to "Prepare code for upcoming ES6 based controller mapping system with module support" does it fit?

Copy link
Member

Choose a reason for hiding this comment

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

For my understanding #2868 enabled the module support. Is it not yet usable?

Oh wow, I forgot about that PR entirely. I guess we technically do have module support, however, we have absolutely 0 docs or already written code that demonstrate this.

Anyway, I have improved the sentences to "Prepare code for upcoming ES6 based controller mapping system with module support" does it fit?

The situation is too complex to describe in two sentences IMO so I don't care how we phrase it exactly. More important is that we document situation ASAP.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no way to load ES6 modules currently.

CHANGELOG.md Outdated
Comment on lines 230 to 231
* Don't allow waveforms to obtain input focus [#4128](https://github.com/mixxxdj/mixxx/pull/4128)
* Inverted scroll wheel waveform zoom direction to mach other applications [#4195](https://github.com/mixxxdj/mixxx/pull/4195)
Copy link
Member

Choose a reason for hiding this comment

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

already part of 2.3.1

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it was merged to main.

Copy link
Member

Choose a reason for hiding this comment

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

My bad, #4195 did only go into main, but #4128 was opened against 2.3 as well (#4134).

Copy link
Member

Choose a reason for hiding this comment

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

yep, the fix was back-ported to 2.3

CHANGELOG.md Outdated
[#2980](https://github.com/mixxxdj/mixxx/pull/2980)
[#3006](https://github.com/mixxxdj/mixxx/pull/3006)
* Logging: Add support for QT_MESSAGE_PATTERN env var [#3204](https://github.com/mixxxdj/mixxx/pull/3204)
* Colored logging console output and other improvements
Copy link
Member

Choose a reason for hiding this comment

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

I dislike the "[...] and other improvements".

CHANGELOG.md Outdated
[#4036](https://github.com/mixxxdj/mixxx/pull/4036)
[#4170](https://github.com/mixxxdj/mixxx/pull/4170)
[#4057](https://github.com/mixxxdj/mixxx/pull/4057)
* Added the Recordbox Hotcue Color Palette [#3746](https://github.com/mixxxdj/mixxx/pull/3746)
Copy link
Member

Choose a reason for hiding this comment

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

already part of 2.3.0

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so, maybe the description was misleading. Improved.

Copy link
Member

@Swiftb0y Swiftb0y Aug 16, 2021

Choose a reason for hiding this comment

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

Same issue as with the other PR. the PR linked is part of main, but the actual feature / commit has also been merged into 2.3 in #3749

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@daschuer
Copy link
Member Author

Done

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

LGTM. Does someone else also want to have a look?

CHANGELOG.md Outdated
@@ -2,10 +2,349 @@

## [2.4.0](https://launchpad.net/mixxx/+milestone/2.4.0) (Unreleased)

* Cover art: Prevent wrong cover art display due to hash conflicts
* Cover art: Add background color for quick cover art preview
### Cover Art ###
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### Cover Art ###
### Cover Art

And everywhere else

CHANGELOG.md Outdated
* Add background color for quick cover art preview [#2524](https://github.com/mixxxdj/mixxx/pull/2524)

### Music Library ###
* Ensure that tracks with an invalid bpm are re-analyzed [#2776](https://github.com/mixxxdj/mixxx/pull/2776)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Ensure that tracks with an invalid bpm are re-analyzed [#2776](https://github.com/mixxxdj/mixxx/pull/2776)
* Ensure that tracks with an invalid BPM are re-analyzed [#2776](https://github.com/mixxxdj/mixxx/pull/2776)

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
[#4143](https://github.com/mixxxdj/mixxx/pull/4143)
* Rekordbox: Save all loops and correct AAC timing offset for CoreAudio [#2779](https://github.com/mixxxdj/mixxx/pull/2779)
* Improve messages during schema migration [#2979](https://github.com/mixxxdj/mixxx/pull/2979)
* Fix "[Library],GoToItem" in tracks table [#3196](https://github.com/mixxxdj/mixxx/pull/3196)
Copy link
Member

Choose a reason for hiding this comment

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

Is this broken in 2.3? Or is this a regression fix? In that case it should not be part of the changelog.

Copy link
Member Author

Choose a reason for hiding this comment

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

This has been backported to 2.3.0 .. removed.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
[#4036](https://github.com/mixxxdj/mixxx/pull/4036)
[#4170](https://github.com/mixxxdj/mixxx/pull/4170)
[#4057](https://github.com/mixxxdj/mixxx/pull/4057)
* Rename Master EQ to Main EQ for an inclusive language [#3868](https://github.com/mixxxdj/mixxx/pull/3868)
Copy link
Member

@Holzhaus Holzhaus Aug 17, 2021

Choose a reason for hiding this comment

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

Make use of inclusive language is already mentioned above

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
* Rename Master EQ to Main EQ for an inclusive language [#3868](https://github.com/mixxxdj/mixxx/pull/3868)
* Improve message when dealing with macOs sandbox [#4018](https://github.com/mixxxdj/mixxx/pull/4018) [lp:1921541](https://bugs.launchpad.net/mixxx/+bug/1921541)
* CONTRIBUTING.md: move content from DocuWiki [#2699](https://github.com/mixxxdj/mixxx/pull/2699)
* Misc. refactorings
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to mention that? I guess no user will know what it means.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was unsure about that. We may remove the whole block. For now I have kept it for completeness.
Ideas?

Copy link
Member

Choose a reason for hiding this comment

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

I think the current entry is not really helpful unless you read all the linked PRs. We could split it up (e.g. "Refactor XYZ", "Fix various compiler warnings", etc.), but I guess this is too much without real relevance to the user.

Or maybe something like "And many more fixes, improvements and cleanups. See the 2.4.0 milestone on GitHub for details."?

Copy link
Member Author

Choose a reason for hiding this comment

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

And many more fixes ..

Without the link is not a good idea, because the user will have hard time to find them among all others already listed above. Should I remove the whole block than, to not make the user curious?

CHANGELOG.md Outdated
@@ -227,7 +566,7 @@
* Fix HID controller output on Windows with common-hid-packet-parser.js
* Fix rendering slow down by not using QStylePainter in WSpinny lp:1530720
* Fix broken Mic mute button lp:1782568
* added quick effect enable button to the control picker menu
* added quick effect enable button to the control er menu
Copy link
Member

Choose a reason for hiding this comment

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

Please revert

Copy link
Member Author

Choose a reason for hiding this comment

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

Ups ..

@daschuer
Copy link
Member Author

Done.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

LGTM, thank you very much for this tedious work.

@Holzhaus
Copy link
Member

Holzhaus commented Sep 8, 2021

Unfortunately there are merge conflicts.

@Holzhaus Holzhaus merged commit 78a547f into mixxxdj:main Sep 8, 2021
@daschuer
Copy link
Member Author

daschuer commented Sep 8, 2021

Thank you for review. A follow up with the latest changes in in progress.

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.

6 participants