[RtM] Documentation Improvements

[citibeth]

When I see ways to improve the documentation, I put it here. Merge at your convenience.

[mathstuf]

Alternatively, you can set the LESS environment variable to contain R (I use LESS=-RIMF locally).

[gartung]

Thanks

[becker33]

If these issues still apply could you please rebase this to the tip of develop?

[tgamblin]

Needs rebase then I can merge.

[adamjstewart]

I think method definitions should always come at the end of Python classes. Can you switch the order of these two and add an empty line between them?

[citibeth]

I think a Spack-specific exception to that rule should be made here. In Spack packages, we look for the URL just before the version numbers + checksums. It should not matter whether the URL is specified with url=... or url_for_version(). I will leave this as-is unless further debate settles it otherwise.

[adamjstewart]

Also, every package that uses url_for_version that is currently in Spack puts the url_for_version method with all of the other methods...

[citibeth]

@tgamblin Any opinions?

The full example in question is as follows:

   class Foo(Package):
       def url_for_version(self, version):
           return 'http://example.com/version_%s/foo-%s.tar.gz' \
               % (version, version)
       version('8.2.1', '4136d7b4c04df68b686570afa26988ac')
       ...

How would this change if we were to enforce url_for_version() below?
Would we need the following? Would it be more / less clear in context of
other examples of Foo that just use url=?

   class Foo(Package):
       version('8.2.1', '4136d7b4c04df68b686570afa26988ac')
       ...
       def url_for_version(self, version):
           return 'http://example.com/version_%s/foo-%s.tar.gz' \
               % (version, version)

On Tue, Aug 23, 2016 at 4:39 PM, Adam J. Stewart notifications@github.com
wrote:

In lib/spack/docs/packaging_guide.rst:

@@ -385,19 +387,28 @@ in the package. For example, Spack is smart enough to download
version 8.2.1. of the Foo package above from
http://example.com/foo-8.2.1.tar.gz http://example.com/foo-8.2.1.tar.gz.

-If spack cannot extrapolate the URL from the url field, or if
-the package doesn't have a url field, you can add a URL explicitly
+If spack cannot extrapolate the URL from the url field by
+default, you can write your own URL generation algorithm in place of
+the url declaration. For example:
+
+.. code-block:: python

• :linenos:

• class Foo(Package):

•   def url_for_version(self, version):
  

•       return 'http://example.com/version_%s/foo-%s.tar.gz' \
  

•           % (version, version)
  

•   version('8.2.1', '4136d7b4c04df68b686570afa26988ac')
  

Also, every package that uses url_for_version that is currently in Spack
puts the url_for_version method with all of the other methods...

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/LLNL/spack/pull/663/files/0736807da5cd4bbc89c8f4cbd54d60d8cf128fc4#r75946143,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB1cdwXTvu9oXxVSYMUmTWU7X8MY5CQmks5qi1qQgaJpZM4H5he8
.

[davydden]

agree with @adamjstewart here, it reads better to have url_for_version below version definitions.

[citibeth]

OK it's rebased.

[citibeth]

I don't agree with the position of url_for_version(), but I just did it because this PR needs to get merged and not held up any longer. @tgamblin can it merge now?

[adamjstewart]

LGTM