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

Remove the 6 year old TODO file #1964

Closed
wants to merge 1 commit into from
Closed

Conversation

Wallacoloo
Copy link
Member

The TODO file in the repository root hasn't been touched in 6 years. We have an issue tracker, so I don't see the need of keeping a to-do file in the actual repository anymore.

Edit: I'll maintain the entries of the to-do list that haven't been implemented / posted to the tracker below. Leave a comment on which ones are safe to be removed:

Unknown if valid or not:

-- resample sample-track-tcos when exporting at different samplerate        
-- select all MIDI devices by default when you bring up the "connect to controller"     
-  window and wait for first event - then uncheck all other MIDI devices that no        
-  events were detected from        
-- load asdlol.mmpz. if you render it without playing it, or if you play it     
-  the first time, you hear unwanted artifacts. (solution: apply automation     
-  before playing)      
-- speed up painting of sampleTCO       
-- when you click and drag a mixer bar, it doesn't click and drag, it sets      
-  absolutely. this is annoying     

Probably still valid issues for which no tracker exists:

-- message to user when importing unsupported MIDI-file (track-count = 0) 

@tresf
Copy link
Member

tresf commented Apr 16, 2015

Agreed, but some of these are valid. We should probably file bug reports for those not currently represented in our tracker. This is a very timely process. I would recommend going line-by-line and crossing them out. People like @musikBear @Sti2nd @Umcaruje @mikobuntu @curlymorphic @badosu and myself could help with this effort, since they're very familiar with the state of our current tracker.

-Tres

@musikBear
Copy link

You mean this one
https://github.com/LMMS/lmms/blob/master/TODO
??
looks messy.. almost like a 'note to self' or 'assented' notes..

@curlymorphic
Copy link
Contributor

I will start by deleting the items i know are done.
If every one does the same in a chain, we can see whats still left.

-- resample sample-track-tcos when exporting at different samplerate        
-- message to user when importing unsupported MIDI-file (track-count = 0)       
-- when you add vestige, have it automatically pop the find VST plugin dialog       
-- select all MIDI devices by default when you bring up the "connect to controller"     
-  window and wait for first event - then uncheck all other MIDI devices that no        
-  events were detected from        
-- load asdlol.mmpz. if you render it without playing it, or if you play it     
-  the first time, you hear unwanted artifacts. (solution: apply automation     
-  before playing)      
-- speed up painting of sampleTCO       
-- copy-pasted automation patterns have to be manually linked back to       
-  their knob for some reason       
-- improve TrackLabelButton: split 80%-20% (80%=name, 20%=button showing a popup        
-  menu with track operations, make the midi input a top-level menu item)       
-- when you click and drag a mixer bar, it doesn't click and drag, it sets      
-  absolutely. this is annoying     
- ````

@Wallacoloo
Copy link
Member Author

@curlymorphic Thanks for going through the list! I edited your list into the PR description so that people can leave comments here just detailing the ones they know are safe to remove.

@musikBear yes, that's the one.

I can't make sense of very many of these. In regards to this entry:

-- when you click and drag a mixer bar, it doesn't click and drag, it sets      
-  absolutely. this is annoying

I tried dragging both the vertical mixer channel bars and the FX slots in the FX chains, and neither of them support dragging. Any idea what this one is referring to? Maybe a feature of the old mixer? Is it safe to delete this entry?

@Wallacoloo
Copy link
Member Author

-- improve TrackLabelButton: split 80%-20% (80%=name, 20%=button showing a popup        
-  menu with track operations, make the midi input a top-level menu item) 

This seems to be referring to this piece of the code (replicated below):

if( ConfigManager::inst()->value( "ui",
                      "compacttrackbuttons" ).toInt() )
{
    setFixedSize( 32, 29 );
}
else
{
    setFixedSize( 160, 29 );
}

setIconSize( QSize( 24, 24 ) );

So the ratio of the icon to the text is currently 85%-15% (24/160 is 15%). A ratio of 80%-20% could be achieved by setFixedSize(120, 29) and keeping the icon size at 24. This seems like a matter of personal preference, and I haven't seen any one else complain about the current icon to text split. Therefore, I am removing this entry - comment if you think it needs more discussion.

@Wallacoloo
Copy link
Member Author

-- copy-pasted automation patterns have to be manually linked back to       
-  their knob for some reason

I just tested that and it appears that this issue has since been solved; removing this item from the list.

@Wallacoloo
Copy link
Member Author

-- when you add vestige, have it automatically pop the find VST plugin dialog

This hasn't been implemented & wasn't on the issue tracker, so I created #1983 for it.

@Wallacoloo
Copy link
Member Author

As far as -- message to user when importing unsupported MIDI-file (track-count = 0):

This condition is detected in mfmidi.cpp:31. Following the call chain, the message isn't logged in that class, but is relayed to a pure-virtual method, Mf_error. The only class that implements that is Alg_midifile_reader in allegrosmfrd.cpp:172, which just logs the error to the console.

Looking further, it looks like LMMS' MidiImport class doesn't bother using that Midi reader for some reason though (so you can probably discard the above information). The only call into the allegro library is, from what I can tell, in creating an Alg_seq in MidiImport.cpp:283. From here, it looks like the only time the number of tracks in the midi file is queried (seq->tracks()) is in setting up the progress display and in the for loop that imports each track.

So from what I can tell, no checks are made to ensure that a Midi file has > 0 tracks. Therefore this issue is probably still valid. But I don't know enough about Midi to be able to create a 0-track midi file and test it.

@tresf tresf mentioned this pull request Jul 21, 2015
@tresf
Copy link
Member

tresf commented Aug 17, 2015

This PR isn't needed since the TODO file was removed via #2210. @Wallacoloo if you'd like, feel free to reopen this as a bug report (or several, depending on your approach).

@tresf tresf closed this Aug 17, 2015
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.

4 participants