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

Rename old style classes and files [wip] #1353

Merged
merged 22 commits into from
Nov 26, 2014
Merged

Rename old style classes and files [wip] #1353

merged 22 commits into from
Nov 26, 2014

Conversation

lukas-w
Copy link
Member

@lukas-w lukas-w commented Nov 26, 2014

Welcome to my great suicidal attempt at renaming all classes and files to fit the new C++ style.
I keep this pull request for checking my changes with Travis and to apologize for the countless merge conflicts lying ahead.

@diizy
Copy link
Contributor

diizy commented Nov 26, 2014

Ok yeah, this is admirable but we should hold this off for a bit... so
much conflicts.

@tresf
Copy link
Member

tresf commented Nov 26, 2014

my great suicidal attempt at renaming all classes and files to fit the new C++ style

Sounds like a mission for a ninja with ninja skills. 👍 Moral support.

@diizy, when you say "hold off", are you asking him not to work on the pull, or "hold off" before merging the pull? Since it's a WIP, he'll be in conflict hell in any scenario without a code freeze, right?

I'm asking not because I question anyone's judgement, just want to learn. :)

@diizy
Copy link
Contributor

diizy commented Nov 26, 2014

On 11/26/2014 05:44 AM, Tres Finocchiaro wrote:

my great suicidal attempt at renaming all classes and files to fit
the new C++ style

Sounds like a mission for a ninja with ninja skills. 👍 Moral support.

@diizy https://github.com/diizy, when you "hold off", are you asking
him not to work on the pull, or "hold off" before merging the pull?
Since it's a WIP, he'll be in conflict hell in any scenario without a
code freeze, right?

I'm asking not because I question anyone's judgement, just want to
learn. :)

Well I'm mainly concerned because there's so much stuff going on right
now... there's the lmms2 branch where I'm trying to start up the efforts
to redesign and clean up LMMS, several people are currently targeting
master with pull requests... lots of things that will conflict.

Then again, there's never a really "good" time for this kind of stuff,
but still...

@lukas-w
Copy link
Member Author

lukas-w commented Nov 26, 2014

Yeah there's really never a good time for this. I know it's gonna be hell, but I think it only gets worse the longer we wait. The more work on lmms2 advances, the more conflicts. Better sooner than later.

Oh and thanks for the moral support. :)

* about_dialog -> AboutDialog
* bb_editor -> BBEditor
* export_project_dialog -> ExportProjectDialog
* setup_dialog -> SetupDialog
* string_pair_drag -> StringPairDrag
* aboutDialog -> AboutDialog
* bbEditor -> BBEditor
* exportProjectDialog -> ExportProjectDialog
* setupDialog -> SetupDialog
* stringPairDrag -> StringPairDrag
I think this class isn't even used, probably safe to remove?
Conflicts:
	src/core/Song.cpp
	src/gui/LfoControllerDialog.cpp
	src/tracks/InstrumentTrack.cpp
@lukas-w
Copy link
Member Author

lukas-w commented Nov 26, 2014

I think I'm done.

@diizy
Copy link
Contributor

diizy commented Nov 26, 2014

Small nitpick here...

I wonder if BBEditor (BBTrack, BBTrackContainer etc.) should properly be
capitalized as:

BbEditor, BbTrack, etc.

By convention, we've usually treated abbreviations as one word, eg. Tco
instead of TCO... so to be consistent, should we maybe use Bb instead of BB?

diizy added a commit that referenced this pull request Nov 26, 2014
Rename old style classes and files [wip]
@diizy diizy merged commit bdbedad into master Nov 26, 2014
@lukas-w
Copy link
Member Author

lukas-w commented Nov 26, 2014

By convention, we've usually treated abbreviations as one word, eg. Tco instead of TCO... so to be consistent, should we maybe use Bb instead of BB?

Ah I didn't know about that convention. I find Tco more confusing than TCO though. The latter makes it obvious that it's an abbreviation.

@lukas-w lukas-w deleted the rename branch November 26, 2014 12:07
@diizy
Copy link
Contributor

diizy commented Nov 26, 2014

On 11/26/2014 02:06 PM, Lukas W wrote:

By convention, we've usually treated abbreviations as one word,
eg. Tco instead of TCO... so to be consistent, should we maybe use
Bb instead of BB?

Ah I didn't know about that convention. I find Tco more confusing than
TCO. The latter makes it obvious that it's an abbreviation.

On the other hand it maybe makes it simpler to remember how to
capitalize identifiers... like, if you remember the name of a
type/variable, but not how it's exactly spelled...

Well, it's not a big issue either way - most editors have autocomplete
these days so I guess the point is moot.

@lukas-w
Copy link
Member Author

lukas-w commented Nov 26, 2014

Well I'm fine with BbTrack and Tco as well, I guess that's more common. Feel free to change that.

@diizy
Copy link
Contributor

diizy commented Nov 26, 2014

Lukas you forgot to rename things in the CSS...

@lukas-w
Copy link
Member Author

lukas-w commented Nov 26, 2014

Nope, did that ad1e495

@lukas-w
Copy link
Member Author

lukas-w commented Nov 27, 2014

Ah, I didn't push that commit to lmms2, sorry. Just merge master and the theme will be fixed.

@diizy
Copy link
Contributor

diizy commented Nov 27, 2014

On 11/27/2014 05:41 PM, Lukas W wrote:

Ah, I didn't push that commit to |lmms2|, sorry. Just merge master and
the theme will be fixed.

No worries, I fixed it already in lmms2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants