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

Add more way to specify path for debug handlers. #281

Conversation

Cjkjvfnby
Copy link
Contributor

I plan to store handlers code in freeorion-utilities repo. This update will help to use config files directly from repo without changes. See example for dumpers in my branch https://github.com/Cjkjvfnby/freeorion-utilities/tree/dump_reader/dump_writer

This changes should be in master too. Should I cherry pick them and made other PR?

@Dilvish-fo
Copy link
Member

I plan to store handlers code in freeorion-utilities repo.... This changes should be in master too.

I'm not really understanding your intent here.

What do you propose putting into the utilities repo? At first glance this this appears to probably be something that would be generally useful to anyone looking to modify the AI. The only current material in the freeorion-utilities repo is some migration code that is there mostly to document what has been done. It may be useful for someone else migrating a project, but is not especially likely to be useful to the FO project again. Although the general intro to that repo is a bit more general, I would expect that anything with ongoing usefulness for working on the AI would be included in the main repo. And if it's in the main repo, I don't see any need for it to also go into the utilities repo.

As for the dumper commits you linked to, could you add a little more explanation to them? For example, will the order dumper be automatically enabled, or does something need to be done to enable it?

@genestack-solomatin
Copy link

I want to use/improve dumpers and AI code in same time. Having them in separate repos helps me a lot. If we merge dumpers to master it will be more easy to handle this situation.

Allow to run debug code from any location still looks for me as good idea.

This line responsible for executing code after generate orders: https://github.com/Cjkjvfnby/freeorion-utilities/blob/dump_reader/dump_writer/dumps.py#L250

To run dumpers you need just run game with config ./freeorion --ai-config ~/freeorion-utilities/dump_reader/dump_writer/dumps.ini (to run it with current master change filename in dumps.ini to absolute path.) Dumps will be saved sot same folder where dumps.py is located.

Dilvish-fo added a commit that referenced this pull request Sep 10, 2015
…or-debug-handlers

Add more way to specify path for debug handlers.
@Dilvish-fo Dilvish-fo merged commit 4bb8c03 into freeorion:release-0.4.5 Sep 10, 2015
@geoffthemedio
Copy link
Member

merged into the release branch?

@Vezzra Vezzra added the category:refactoring The Issue/PR describes or contains an improved implementation. label Sep 11, 2015
@Vezzra Vezzra added this to the Next release milestone Sep 11, 2015
@Vezzra
Copy link
Member

Vezzra commented Sep 11, 2015

merged into the release branch?

@Dilvish-fo?

Dilvish-fo added a commit that referenced this pull request Sep 11, 2015
 from Cjkjvfnby/add-more-way-to-specify-path-for-debug-handlers
@Dilvish-fo
Copy link
Member

Gah, sorry, I was double checking for anything that might be waiting on me so I could clear them out, and had forgotten this PR was on the release branch

I've now reverted the mistaken merge commit, and instead cherry picked the commits into the master development branch. I've also confirmed that there are no other open PRs into the release branch so no worries about anyone somehow repeating this blunder.

I propose taking an extra step to clean this up-- do a hard reset of the release branch HEAD back to the commit with the 0.4.5 label (locally is easy, then using push --force to impose that on the Github repo). Normally we wouldn't want to muck with history like that, but since at this point no one should really be doing more development on the 0.4.5 release branch it seems reasonable. @adrianbroher -- care to advise on the wisdom (or lack thereof) of such a move?

@Cjkjvfnby Cjkjvfnby deleted the add-more-way-to-specify-path-for-debug-handlers branch September 13, 2015 08:37
@Vezzra
Copy link
Member

Vezzra commented Sep 13, 2015

I propose taking an extra step to clean this up-- do a hard reset of the release branch HEAD back to the commit with the 0.4.5 label (locally is easy, then using push --force to impose that on the Github repo). Normally we wouldn't want to muck with history like that, but since at this point no one should really be doing more development on the 0.4.5 release branch it seems reasonable.

As our release management lead and kind of "maintainer" of the release branch I very much approve of this suggestion. I'd strongly prefer that solution than to leave the commit and its reversion in. As you said, it's not to be expected that anyone has done any work on the release branch anyway, and if we give a fair warning on the forum, that should be sufficient.

So, feel free to proceed (unless you want give Marcel some more time to add his 2 cent) 😃

@Vezzra
Copy link
Member

Vezzra commented Sep 16, 2015

Ok, as there have been no objections to the suggested hard reset, I'm going to do that tomorrow. Posted a respective warning on the forum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:refactoring The Issue/PR describes or contains an improved implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants