Skip to content

Conversation

@tshort
Copy link
Contributor

@tshort tshort commented Aug 6, 2018

Tabs are causing formatting issues on GitHub and in my local editor. This PR is a mass search and replace. There are still inconsistencies with indentation, but this helps.

@ufechner7
Copy link

Do you think two spaces is a good choice? I am more used to four spaces.

@tshort
Copy link
Contributor Author

tshort commented Aug 6, 2018

Most files here use two spaces for indentation. But, there are many tabs sprinkled in several files. Replacing these with two spacings generally keeps the indentation consistent.

(I also prefer four spaces for indentation, but I've worked on repos that used two.)

@ufechner7
Copy link

So I wait for the decision of the repo-owner.

@tshort
Copy link
Contributor Author

tshort commented Aug 6, 2018

Two vs. four vs. something else should be another issue. This should be a noncontroversial cleanup.

@HildingElmqvist HildingElmqvist merged commit b56f4c7 into ModiaSim:master Aug 7, 2018
@HildingElmqvist
Copy link
Contributor

I made a mistake of merging. There were many errors in the tabs branch. I did not pay enough attention since large diffs are not presented in the github window.

Tom, could you help undo the merge?

@tshort
Copy link
Contributor Author

tshort commented Aug 7, 2018

I'm sorry about that, @HildingElmqvist. I must have goofed something. If we get Travis set up for CI testing, it'll help avoid this type of thing (we need to register ModiaMath first). We can tell if a PR will mess up the package tests.

I'm not a git or github expert. It looks like you can use GitHub to revert a PR. See here:

https://help.github.com/articles/reverting-a-pull-request/

If that doesn't work, a manual way to do that is to revert a local branch to the last known good version. Then, you can submit that as a PR. It looks like the last good version is 90f52e9.

Git is complicated and confusing, but the good news is that it's hard to lose information, so we should be able to backtrack.

@tshort
Copy link
Contributor Author

tshort commented Aug 7, 2018

I can also take a look to see if I can fix what I broke. Will look tonight.

@tshort
Copy link
Contributor Author

tshort commented Aug 7, 2018

Looks like the damage was limited to one file. Working on a PR to fix it now.

@tshort tshort mentioned this pull request Aug 7, 2018
@tshort
Copy link
Contributor Author

tshort commented Aug 7, 2018

FYI, the way I fixed it was to revert the damaged file (in a separate branch):

git checkout -b tabs-to-spaces-fix
git checkout  90f52e97dd08c0f73f48ad6e1f18979e57d6c15f src/symbolic/DAEquations/SymbolicTransform.jl 

I ran the package tests in Julia to make sure they passed. I edited SymbolicTransform.jl to replace tabs with two spaces. Lastly, I committed the change and submitted a PR.

@tshort tshort deleted the tabs branch August 18, 2018 18:11
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.

3 participants