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.
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
improved build workflow #272
improved build workflow #272
Changes from all commits
10b30cd
fedb07f
2399c97
c30d38b
d3cb272
613cc46
376e852
4ae07c1
a8963ee
76c6bbd
bccd4f1
6dcb088
e8829e0
2c2dba9
4680c87
cb0d3f7
d307616
1f70ab9
e40653a
18e1b67
c35554a
fe93cb3
5da72ec
accb53b
b9a2606
40b1c90
86993ee
2c8e2b0
f103b5c
756d6f7
1510369
74bd3bf
0566b17
4618fb1
84acafb
4f50e11
e4f4c57
2eb588e
509ffd2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless running from Batch is right around the corner, worth keeping this link or moving its contents here. E.g. you still need to know how to provision EC2 instance and setup swap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was a link to an obsolete schema spec, which I replaced with the correct link. It was not a build process doc, which has never been part of this README (and probably should not IMHO given that it contains internal infra info).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ps. once I get all this landed, and refined, I will update the "manual build process" doc, not linked here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be useful to provide an example config.yaml that can be copied as necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure. later info in this doc points the user at the build_state.py file. And by default you should not specify a config file if doing the standard census build. It is only for dev reasons that you would use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
example added, plus verbiage about not needing to provide a config for the
standard
census buildThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to
dev-config.yaml
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather leave as-is. It is the config, for any purpose. I just have defaults set so that it isn't required in our current workflow, but I can easily imagine situations where it is used in the future.
that said, if you feel strongly, I'm not going to force this issue :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move into a Makefile target for convenience?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent idea.