Pull request for #77: Replace <*> with <source file name> in <sconscript_*.log> #80
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
#82 has some fixes for the tests of build_table. We'll hold off here until that goes through to avoid redundant work. |
|
Tests run fully now, @arosenbe. Thanks! |
arosenbe
reviewed
Jul 24, 2017
There was a problem hiding this comment.
Nice work @stanfordquan!
Can you also remove the stray mark here
gslab_scons/builders/build_lyx.py
Outdated
| log_file = target_dir + '/sconscript.log' | ||
| try: | ||
| log_ext = '_%s' % env['log_ext'] | ||
| except: |
There was a problem hiding this comment.
Specify the error (IndexError, I think), applies to all builders.
There was a problem hiding this comment.
Sure, but that broke a bunch of unittests that feed weird objects as Env
gslab_scons/builders/build_lyx.py
Outdated
| log_ext = '_%s' % env['log_ext'] | ||
| except: | ||
| log_ext = '' | ||
| log_file = target_dir + ('/sconscript%s.log' % log_ext) |
There was a problem hiding this comment.
Don't mix string concatenation (+) with sprintf style string construction. I'd switch everything over to sprintf, or do it in two line, first constructing the log file with splrintf and then concatenating it to target_dir. Applies everywhere.
There was a problem hiding this comment.
We also might consider using os.path.join
added 2 commits
July 24, 2017 14:15
|
@arosenbe I responded to tyour PR but that broke a number of unittests that feed weird objects as |
|
@arosenbe I fixed the unittests. |
Merged
qlquanle
pushed a commit
that referenced
this pull request
Jul 27, 2017
* Pull request for #28: gslab_scons restructure (#47) * #28 modify check_code_extension() * #28 make distinction between `target_dir` and `log_dir` for all but `build_stata()`, add code flow comments in builder * #28 minor change, `return None` switched to `return False` for logical check * #28 add informative BadExecutableError messages * #28 merging remote to local * #28 add single log file + appending mechanism * #28 refactor build_stata() * #28 add formatting for logs * #28 fix typo in docstring * #28 update error messages and tests * #28 add mode check in start_log() * #28 change formatting in master tests to match work in this task * #47 dev commit to work from home * #47 rework logging different appending mechanism from builder logs to sconstruct.log * #28 respond to PR * #28 make pretty * #28 fix lyx pdf issue * #28 respond to more comments * #28 fix typo * Pull request for #59: Fix size warnings (#60) * #59: Redesigned size testing * #59: debugging * #59: additional work on list_ignored_files() * #59: Additional testing progress * #59: Brought tests to 100% coverage * change * #60: responded to PR * 60: response to PR * #59: brought size_warning tests to 100% coverage * #59: removed sys.exit() and raw_input() from size warning function * #59: scons debrief function * #59: altered size warnings * #59: small revisions to size warnings * #60: misc changes * Add note about sudo * Update README.md * Pull request for #43 in Template: Configuration tests (#66) * Migrates configuration tests * Preliminary tests setup * Additional tests * More edits * Adds more tests * Adds more tests * Adds more tests * Adds more tests * Adds more unit tests * Adds more unit tests * Fixes load_yaml_value tests * Dev commit * Dev commit * Brings coverage to 100 percent * Changes name * Edits tests for dropping --sf command line call * Changes name back * Fixes name in function * #66 Implements prelim PR comments * #66 Other PR comments * #66: Standardised exception classes * #66: updated some tests * #66: misc changes to tests * #66: some more changes * #66: changed error message * #66: removed extra newline from yaml creation * #66: aesthetic changes * #66: updated version number to 4 * Pull request for gslab-econ/template#45: Standardise stata_executable (#67) * gslab-econ/template#45 Changed to 'stata_executable' * Fixed a comment... * #67: Responses to PR * #67: Response to PR * Pull request for #58: Prepare for 4.0 (#65) * #58 cosmetic changes * Issue 58 - implement comments to builders 1) Fix R builder command line argument issue. Will test if issue persists for other builders. 2) Time-log failure is now kinder (does not block completion of SConstruct, just raise a noisy warning) * #58 small changes during merge * Pull request for #69: Update tablefill builder with 4.0 logging machinery (#70) * #69 add logging functionalities * #69 decrease clutter in sconscript.log * #69 prettify output * Pull request for #75: Print traceback for python builder (#76) * #75 cat \nTraceback to error message in Python builder * #75 remove new line * #75 fix crashing log.py if sconscript.log is empty * Pull request for #68: Remove Google Drive from default release (#72) * issue #68 - Fix broken imports in test - Move `read_yaml_value` to `misc` from `config` - Fix appropriate tests broken because of above move * #68 accommodate atexit implementation of state_of_repo * #68 fix numargs * #68 - instead of _ for user-config.yaml * #68 fix regex error in _release_tools * #68 respond to PR * #68 move check_and_expand_cache_path to misc.py, rename check_and_expand_path * #68 response to PR about release paths * #68 remove prompt for github token * #68 respond to PR * #68 add logical gate to use getpass.getpass for github_token * Pull request for #78: Updates readme (#79) * #78 Updates readme * Update README.md * Pull request for #73: Allows tablefill to use tex documents (#82) * #73 Preliminary implementation * #73 Updates info * #73 add newline after log for easy viewing * Pull request for #77: Replace <*> with <source file name> in <sconscript_*.log> (#80) * #77 add optional log extension through way of log_ext in ENV * #77 revert matlab reordering * #77 fix GitKraken merge typos * 77 add newline back in tablefill logging * #77 fix unittests * #77 respond to PR * #77 remove stray backslash * #77 fix broken unittests that allow dumb input as env * #83 add coverage to dependencies (#84) * Pull request for #85: Find way to speed up log.py (#87) * #85 log search with CL tool * #85 new log search works for mac * #85 get logging working on windows * #87 #85 var named cmd -> command, filter + map removes null paths, new warning msg * Update log.py * Update README.md * Update README.md * Update release.py * #88 format release instructions * #88 remove repeated word in readme
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@arosenbe I added a step in builders for the user to specify a bit in the sconscript_*.log as mentioned in the title. The user specify an argument in
env.BuildSomethingaslog_ext = *. It should be fairly obvious what happens. There's a PR intemplateas a companion to this.I would appreciate it if you can help out updating the unittests since you're the Mock expert.
Thanks!