Skip to content

Conversation

@notahat
Copy link
Owner

@notahat notahat commented Dec 8, 2019

I've gone back to the drawing board on #2 and #4, because I wasn't comfortable with the memory management changes from #2. At best they'll leak memory, and I suspect they'll cause crashes in edge cases.

I also spotted some changes that, while they fixed warnings, left holes in the associated functionality.

So I've gone back to master, and started fixing and pulling in changes bit by bit. Specifically, I:

  • set the project to build for macOS 10.15, so we get all the latest deprecation warnings...let's make this thing behave well for the next 10 years!
  • converted the original nib files to xib files
  • fixed all the value conversion warnings properly, without type-casting away precision
  • fixed almost all of the semantic warnings

There are still lots of deprecation warnings, some of which simply need code pulled across from #4, and some of which are trickier.

In particular:

  • I'd like to use the secure coding stuff for reading and writing files
  • I'd like to figure out if we can replace the KeyTableView with a view based TableView

Other things still do to:

  • Fix display issues when editing the virtual endpoints
  • Set the build number correctly

Things to test:

  • saving and loading
  • copying and pasting
  • undo
  • all the places NSAlert is used

Other puzzles:

  • AddInstanceForFactory warning on start
  • Do I need "Weak References in Manual Retain Release" on?

/cc @danomatika

@danomatika danomatika mentioned this pull request Dec 9, 2019
@danomatika
Copy link
Contributor

danomatika commented Dec 9, 2019

I agree to all, however I'd still recommend moving to ARC. The CoreMIDI stuff could use a second glance but everything else is simply removing retain/release and [super dealloc]. There may be a few places where a manual release by setting to nil is warranted.

@notahat notahat mentioned this pull request Dec 11, 2019
13 tasks
@notahat notahat merged commit 26c85bd into master Dec 14, 2019
@notahat notahat deleted the modernise-take-two branch December 14, 2019 03:22
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