Repository navigation
Initial layout - #1
Conversation
|
This looks good, although the pending changes in setup.py on astropy are something we definitely want in here, too. That can be pulled in separately, though, if you'd rather get this merged now. But we'll probably want to be cautious about letting it out much until those fixes are done. |
There was a problem hiding this comment.
Minor point: I added some other things in the astropy .gitignore recently that we may want to merge here. Python 3 support (pycache) and emacs backup files (*~).
There was a problem hiding this comment.
I've updated to the lastest version of the .gitignore file in the core astropy repository
|
In most of my inline comments above I was assuming that all affiliated packages would depend on astropy and that astropy would be installed and available during build time. I realize now that may not be a correct assumption, but it should probably be clarified. If I'm not correct, then we should ensure that changes to many of these files that are not intended to be edited (pytest, big chunks of conf.py, sphinxext directory etc.) remain easy to merge in from the package-template project, as they are likely to be updates made centrally that we want to easily push out to all affiliated packages. Instructions should probably be included about how to do that. |
|
That pull request you issued mostly addresses this already, but I think it's fair to assume that anyone who plans to use the affiliated package template can expect that people already have astropy installed. If this ends up a problem, it should be fairly easy to just move the associated helpers over to an affiliated package. Also, I think the best way to deal with the problem of pushing out later changes is to make it clear that the procedure for making a new affiliated package is something like: git clone [email protected]:astropy/package-template.git And then that at any time they can just pull updates from the package template and it will make the appropriate changes (at least, generally). |
|
Do we really need people to clone the template package repository? We could just require them to use the same layout, and not care if they actually clone the 2-3 initial commits, right? (of course they can if they want, but does it really matter?) |
|
They might want to because then they can easily merge in later changes made here. It might still be fine if they make a copy of this as it stands and make sure their first commit is an exact copy of this repo's current state... The main problem is that git has to know what has been moved from a given directory in this repo to a new directory in an affiliated package's repo. And just generally the revision history looks much more readable if they all come from a common ancestor. |
|
Of course none of that matters if we don't expect people to ever pull from this repo again once they've started their affiliated package... but I don't think we want to count on that. |
|
My thought was more that it's up to them to have the layout agree with the affiliated package layout once they submit it for a pull request, and it doesn't really matter how they achieve that. When we review the pull request, we can just examine whether all the required elements are there. Of course, we can include guidelines for how to do it via cloning the template-package repository if they want to, but I think it's something we don't need to worry about too much as what matters for us is the end result. |
|
Right, I didn't mean any of this as a requirement, just a recommendation like "if you clone first as describe here, it will make it easier for you to merge later changes from the template if you want to. But if you don't want to, that's fine too, as long as everything works later if you want to merge with the core." |
|
Sure, although I'll wait until we've gotten this pulled in and the setup.py stuff Erik B. is planning is done, as that might change some of the exact syntax. |
|
I agree with @eteq -- it will save a lot of headache if people a) use git and b) clone from the template so changes to the template can be more easily merged in later. We shouldn't require it -- if developers want to use another VCS they can, but they're "on their own" and may have to manually merge in template changes later. Of course, my suggestions above -- to use code from the astropy core whenever possible (for sphinx extensions, py.test etc) -- ameliorates some of that anyway. The less real code in the template itself, the less need there will be to update it. |
|
I've addressed most of the issues above. I will reply to each inline comment. |
|
Once this is ready to merge, I propose to rebase and squash all commits into one, since all affiliated packages will copy this history, and there is no point in having big files like py.test added and removed in the history. Also, a single commit for setting up the affiliated package layout will look cleaner. What do you think? |
|
I think the squashing sounds like a good plan. I see two near-term updates that we will probably want to push out to this project when they are ready:
|
|
+1 on squashing. @eteq: We are already using autodoc in master. What is the change you're referring to? (More instructions never hurts, of course). |
|
Somehow I miss out on all this stuff until rather late... is there something I can do (that I've somehow missed) to get notifications (preferably in digest form) of all Astropy happenings on github? I don't usually get anything until I'm mentioned somewhere. |
|
@mdboom - we're using autodoc, but haven't yet set up autosummary to create the reference API docs. That's next on my todo list after the config stuff (along with some other doc-related fixups) |
|
@iguananaut - I'm not sure exactly how to do this. If you're "watching" the repo, you should get this stuff in your notification center, and I think there's a way to convert those notifications into an email digest, but I'm not sure how... If you're a team member for the repo you get updates, though, and we'll add you to keep an eye on FITS, so at least for this particular repo it'll soon be a moot point. |
|
Looks good to me! |
|
One final change - if cython is not installed, it's better to have the cython test fail than no test run at all (which is what was happening before). We could have a skip statement instead, but I think that in this case it makes more sense the way it is, otherwise it would silently not test the cython version. |
|
Ok, so once I merge, I squash the history on master into a single commit with message 'Initial commit'? |
|
Sounds good, although perhaps with a slightly more specific message so people know its from the template - e.g. "initial commit of affiliated package template" |
|
Should |
|
Probably... it doesn't really matter given that it's just the directory name, but that's certainly more consistent. |
|
All right, sorry for the bunch of minor commits - the only thing that needed fixing was that in I just tested this all over again, and it seems to work. Ok to merge? |
|
I'm happy with it - feel free to merge! |
|
Thanks to all of you for your help! I have merge the pull request and squashed the history. GitHub seems to still show the old commit messages on the front page, but I think that is probably just a cache issue. I think this is ready to advertize! @eteq - do you want to send a message to the list, or shall I? |
|
@taldcroft - you might want to start over for pyidlastro with the proper template package, since we've now squashed the history into a single commit (to avoid having all template packages have 58 redundant commits). |
|
Sorry it's taken me so long to respond -- I've been trying to hide from my email for a few days... I'll pull this down and give it a go myself for specutils. |
|
@adrn - now that we've merged, make sure you start from master (https://github.com/astropy/package-template) rather than from my fork. |
|
@astrofrog - either way is fine with me... given that the commit has your name on it, it might make sense to come from you, but I'm happy to write it if you'd rather not for whatever reason - just let me know |
update readme with a bit more info
Update project url default link
I've tried to strip out all the core-specific stuff. Does this look fine for a template package? Note that people should not start affiliated packages by cloning from here, but just copying over the contents to a new repository. Let me know what you think.