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.

Sign up for GitHub

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

Jump to bottom

Fixed Tileset save causing crash #12708 #12732

Closed
wants to merge 28 commits into from
Closed

Fixed Tileset save causing crash #12708 #12732

wants to merge 28 commits into from

Conversation

CalebMRichardson
Copy link

@CalebMRichardson CalebMRichardson commented Nov 7, 2017 •
edited
Loading

Fix crash when converting empty scene to TileSet

Closes #12708.

@mhilbrunner
Copy link
Member

You probably want to clean that up by squishing those commits :)

@CalebMRichardson
Copy link
Author

Oops. I'll look up how to do that. Thanks!

@akien-mga akien-mga added this to the 3.0 milestone Nov 8, 2017
@akien-mga
Copy link
Member

Indeed, see our documentation about rebasing branches to squash commits. Please also amend the resulting log to be more descriptive, "Fixed issues #" doesn't say anything to someone reading the changelog. A log like this would be better:

Fix crash when converting empty scene to TileSet

Closes #12708.

@CalebMRichardson
Copy link
Author

@akien-mga Thanks for the docs. I've updated the log to be more descriptive and I believe I squashed the commits correctly.

@CalebMRichardson
Copy link
Author

@akien-mga sorry to bother you could you point me in the right direction on why travis-ci keeps failing me?

capture

Caleb Richardson and others added 7 commits November 8, 2017 15:03
akien-mga
akien-mga reviewed Nov 9, 2017
View reviewed changes
editor/editor_node.cpp Outdated
@@ -1932,6 +1932,15 @@ void EditorNode::_menu_option_confirm(int p_option, bool p_confirmed) {

} break;
case FILE_EXPORT_TILESET: {

Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary indentation here (empty line), that's why Travis rejects it.

@akien-mga
Copy link
Member

You should rebase your branch as you have 5 commits for what should be a single one. If you're not familiar with this workflow, I suggest to read the whole page about our PR workflow.

akien-mga and others added 11 commits November 9, 2017 16:30
@akien-mga
Merge pull request godotengine#12780 from karroffel/dictionary-crash
c7cd89c 
fix dictionary constant access crash
@Chaosus
Enable asset store for godot 3.0
423f299
@akien-mga
Merge pull request godotengine#12778 from Chaosus/enableassetstore
d7add01 
Enable asset store for Godot 3.0
@akien-mga
Merge pull request godotengine#12746 from rraallvv/unified_headers_fix
e4effb4 
unified headers fix (master)
@rraallvv
add missing distutils.version.LooseVersion import
7957bbf
@reduz
Make video mode initialization more intuitive, fixes godotengine#12022
d09160a
@akien-mga
Merge pull request godotengine#12782 from rraallvv/unified_headers_fix
2507818 
add missing distutils.version.LooseVersion import
@reduz
fixes to initialization order
5cb1d06
Fixed issue godotengine#12708 Empty Scene Tileset causing crash
98e6d8c
Merge branch 'bug_checking' of https://github.com/CalebMRichardson/godot
74663ba 
 into bug_checking
Fixed Tileset Save Causing Crash godotengine#12708 (Fix to respect Cl…
966885d 
…ang)
@akien-mga
Copy link
Member

I'll cherry-pick the relevant commit manually, it will be simpler :)

@CalebMRichardson
Copy link
Author

@akien-mga haha thanks! Sorry for the extra work I kinda got myself stuck and have been trying to panic fix it. I'll be more cautious in the future. Thanks again!

akien-mga pushed a commit that referenced this pull request Nov 9, 2017
@akien-mga
Fix crash when converting empty scene to TileSet
a607e61 
Closes #12708.
Cherry-picked from #12732.
@akien-mga
Copy link
Member

Merged as a607e61.

@akien-mga akien-mga closed this Nov 9, 2017
@akien-mga
Copy link
Member

No problem, git is a tough beast and we all had such weird PRs with tons of unrelated changes a few times :D
If you follow the process outlined in the documentation while using the Git command line, you should be safe (GIt GUIs often use different methods than the CLI behind a common name such as "fetch" or "rebase", so we don't recommend using them as they hide the complexity and cause weird issues).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akien-mga akien-mga akien-mga left review comments

Assignees
No one assigned
Labels
bug topic:editor
Projects
None yet
Milestone
3.0
Development

Successfully merging this pull request may close these issues.