Skip to content
This repository was archived by the owner on Aug 11, 2023. It is now read-only.

Initial layout - #1

Merged
astrofrog merged 58 commits into
astropy:masterfrom
astrofrog:initial-layout
Dec 22, 2011
Merged

astrofrog merged 58 commits into
astropy:masterfrom
astrofrog:initial-layout

Conversation

@astrofrog

Copy link
Copy Markdown
Member

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.

@eteq

eteq commented Oct 25, 2011

Copy link
Copy Markdown
Member

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.

Comment thread .gitignore Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (*~).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated to the lastest version of the .gitignore file in the core astropy repository

@mdboom

mdboom commented Oct 25, 2011

Copy link
Copy Markdown
Contributor

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.

@eteq

eteq commented Oct 26, 2011

Copy link
Copy Markdown
Member

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
git mv packagename
... edit files ...
git commit -am "started "
git push
git branch --set-upstream master

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).

@astrofrog

Copy link
Copy Markdown
Member Author

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?)

@eteq

eteq commented Oct 26, 2011

Copy link
Copy Markdown
Member

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.

@eteq

eteq commented Oct 26, 2011

Copy link
Copy Markdown
Member

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.

@astrofrog

Copy link
Copy Markdown
Member Author

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.

@eteq

eteq commented Oct 26, 2011

Copy link
Copy Markdown
Member

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."

@astrofrog

Copy link
Copy Markdown
Member Author

@eteq that makes sense - do you want to prepare a pull request for the core package which includes those recommended guidelines for starting affiliated packages?

@mdboom - thanks for all the comments! I'm going to implement these changes

@eteq

eteq commented Oct 26, 2011

Copy link
Copy Markdown
Member

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.

@mdboom

mdboom commented Oct 26, 2011

Copy link
Copy Markdown
Contributor

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.

@astrofrog

Copy link
Copy Markdown
Member Author

I've addressed most of the issues above. I will reply to each inline comment.

@astrofrog

Copy link
Copy Markdown
Member Author

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?

@eteq

eteq commented Nov 7, 2011

Copy link
Copy Markdown
Member

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:

  • Whatever mechanism we end up using to specify how to build C-extensions. (this probably doesn't matter as much because I don't imagine too many C-extension affiliated packages at least right now)
  • The documentation framework that uses autosummary/autodoc, and instructions on how it should be used.

@mdboom

mdboom commented Nov 7, 2011

Copy link
Copy Markdown
Contributor

+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).

@embray

embray commented Nov 7, 2011

Copy link
Copy Markdown
Member

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.

@eteq

eteq commented Nov 7, 2011

Copy link
Copy Markdown
Member

@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)

@eteq

eteq commented Nov 7, 2011

Copy link
Copy Markdown
Member

@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.

@eteq

eteq commented Dec 22, 2011

Copy link
Copy Markdown
Member

Looks good to me!

@astrofrog

Copy link
Copy Markdown
Member Author

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.

@astrofrog

Copy link
Copy Markdown
Member Author

Ok, so once I merge, I squash the history on master into a single commit with message 'Initial commit'?

@eteq

eteq commented Dec 22, 2011

Copy link
Copy Markdown
Member

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"

@astrofrog

Copy link
Copy Markdown
Member Author

Should newpackage in README.rst be yourpkg?

@eteq

eteq commented Dec 22, 2011

Copy link
Copy Markdown
Member

Probably... it doesn't really matter given that it's just the directory name, but that's certainly more consistent.

@astrofrog

Copy link
Copy Markdown
Member Author

All right, sorry for the bunch of minor commits - the only thing that needed fixing was that in docs/conf.py, where the whole block of code with packagename had to be changed to yourpkg.

I just tested this all over again, and it seems to work. Ok to merge?

@eteq

eteq commented Dec 22, 2011

Copy link
Copy Markdown
Member

I'm happy with it - feel free to merge!

@astrofrog
astrofrog merged this pull request into astropy:master Dec 22, 2011
@astrofrog

Copy link
Copy Markdown
Member Author

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?

@astrofrog

Copy link
Copy Markdown
Member Author

@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).

@adrn

adrn commented Dec 22, 2011

Copy link
Copy Markdown
Member

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.

@astrofrog

Copy link
Copy Markdown
Member Author

@adrn - now that we've merged, make sure you start from master (https://github.com/astropy/package-template) rather than from my fork.

@eteq

eteq commented Dec 23, 2011

Copy link
Copy Markdown
Member

@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

astrofrog pushed a commit that referenced this pull request Aug 14, 2014
update readme with a bit more info
Cadair referenced this pull request in Cadair/package-template Feb 9, 2018
Update project url default link
crawfordsm pushed a commit that referenced this pull request Mar 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants