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

Russian translation and setup_path() rework #23

Closed
wants to merge 22 commits into from
Closed

Russian translation and setup_path() rework #23

wants to merge 22 commits into from

Conversation

Alex-1000
Copy link
Contributor

Didn't want it to be together in a single pull request but I made epic branching fail, so here we are.
setup_path() now just loops on custom directory if user says no to installing in sourcemod and current directory, waiting for user to input directory and say yes

@Technochips
Copy link
Member

im sorry but this entire commit history looks hellish

tf2c_downloader.py Outdated Show resolved Hide resolved
@Technochips
Copy link
Member

Technochips commented Aug 2, 2022

keep one feature per pull request. don't commit unrelated stuff right after posting a pull request. this is just very confusing to review and the commit history is awful to look at.

consider making new forks off the current branch, and redo the features (copy paste?), then make a pull request for each individual forks/features. also don't bother merging 'main' back to your branch unless we tell you to do it.

as far as i can see, this pull request is identical to #19 (same repo) and that one hasn't been able to get pulled because(?) of the confusing commit history, except this one not only tries to fix the same issue that ended up being fixed in a commit in 'main', but tries to do at least three other things.

@Technochips Technochips closed this Aug 2, 2022
@Alex-1000
Copy link
Contributor Author

So...
Why you closed this pull request?
You and Chloe already told it in Discord before, and #19 was closed because fix in fork main was replaced with code from tf2classic:main (and I didn't like the fix at the time anyway).
There is no new information in your comment and you just close pull request with hours of work.

@chloecormier
Copy link
Collaborator

The code and the work is fine and appreciated. But we would also appreciate if you could split the work into separate pull requests.

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