all files should have standard header with standard keywords

[Dan MacNeil]

Almost every text file in version control should have a header like:

# LICENSE: GNU Affero General Public License v3
# COPYRIGHT: Community Software Lab
# CONTACT: <email address hidden>
# PROJECT: https://launchpad.net/mvhub/

# PURPOSE: This file exists to demo a header
# It should be possible for a PURPOSE
# line to span multiple lines.

This header should be in the first 10 lines of the file. (new 2010-07-24 )

Files with complete POD should maybe not get this header.

Other or no comment styles should be possible. Like:

<!--

- -->

A test should be added:

               app-mvhub/t/header_doc.t

This test should fail if any (?) file lacks the header w/ the needed keywords.

lib-mvhub/t/lib/TestHelper.pm may have useful routines.

For a routine that checks to see if a string is found in a file see:

         MVHub::Utils::Setup::_string_already_in_file()

the routine is found in:

          lib-mvhub/lib/MVHub/Utils/Setup.pm

If this routine is useful, remove the _ and export it.

see also:

app-mvhub/t/required_comments_present.t

...which may have useful logic.

To write this test, you'll

    have to get a list of files in the project
       ---see the routines in TestHelper.pm

    exclude some files (binary)
   check each file for the presence of all required header comments.
   fail the test if they aren't there.

man file
perldoc TestHelper

It is probably also worth writing a script to automatically add header to files that are missing them as it will be tedious to do this by hand.

[troylamonte (troylamonte)]

hey Dan Whats up, this is Troy. We made it to providence, RI, we are now in
the process of getting a place and all our benefits switched to this area.
Looking forward to working with you again. will write as soon as we are
settled.

On Mon, Jul 5, 2010 at 4:27 PM, Dan MacNeil <email address hidden> wrote:

> ** Description changed:
>
> - -----BEGIN PGP SIGNED MESSAGE-----
> - Hash: SHA1
> -
> - affects mvhub
> - assignee troylamonte
> - importance low
> - status confirmed
> - tag refactor
> - done
> -
> Almost every text file in version control should have a header like:
>
> # LICENSE: GNU Affero General Public License v3
> # COPYRIGHT: Community Software Lab
> # CONTACT: <email address hidden>
>
> # PURPOSE: This file exists to demo a header
> # It should be possible for a PURPOSE
> # line to span multiple lines.
>
> Files with complete POD should maybe not get this header.
>
> Other or no comment styles should be possible.
>
> <!--
>
> - -->
>
> A test should be added:
>
> - app-mvhub/t/keywords_found.t
> + app-mvhub/t/header_doc.t
>
> - This test should fail if a file lacks the header w/ the needed keywords.
> -
> - For ugly code that tests for similar things:
> -
> - http://doc.bazaar.canonical.com/plugins/en/keywords-plugin.html
> + This test should fail if any (?) file lacks the header w/ the needed
> + keywords.
>
> lib-mvhub/t/lib/TestHelper.pm may have useful routines.
>
> + For a routine that checks to see if a string is found in a file see:
>
> - -----BEGIN PGP SIGNATURE-----
> - Version: GnuPG v1.4.9 (GNU/Linux)
> - Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
> + MVHub::Utils::Setup::_string_already_in_file()
>
> - iEYEARECAAYFAktw0cIACgkQLzI3mETyffzcIgCdHYPxGoeMM25Y4UcBZ6py0iv3
> - PpUAn376eI2WHStIysE/E9J9J4TDTIxC
> - =osuJ
> - -----END PGP SIGNATURE-----
> + the routine is found in:
> +
> + lib-mvhub/lib/MVHub/Utils/Setup.pm
> +
> + If this routine is useful, remove the _ and export it.
> +
> + see also:
> +
> + app-mvhub/t/required_comments_present.t
> +
> + ...which may have useful logic.
> +
> + It is probably also worth writing a script to automatically add header
> + to files that are missing it.
>
> --
> all files should have standard header with standard keywords
> https://bugs.launchpad.net/bugs/519126
> You received this bug notification because you are a bug assignee.
>
> Status in MVHub, a directory of social services: Confirmed
>
> Bug description:
> Almost every text file in version control should have a header like:
>
> # LICENSE: GNU Affero General Public License v3
> # COPYRIGHT: Community Software Lab
> # CONTACT: <email address hidden>
>
> # PURPOSE: This file exists to demo a header
> # It should be possible for a PURPOSE
> # line to span multiple lines.
>
> Files with complete POD should maybe not get this header.
>
> Other or no comment styles should be possible.
>
> <!--
>
> - -->
>
> A test should be added:
>
> app-mvhub/t/header_doc.t
>
> This test should fail if any (?) file lacks the header w/ the needed
> keywords.
>
> lib-mvhub/t/lib/TestHelper.pm may have useful routines.
>
> For a routin...

[Dan MacNeil (omacneil)]

#############
COMMENTS FROM expired merge request for
lp:~himabindu-sanagavarapu/mvhub/ADD_HEADER_TO_MISSING_FILES
#############

This is a huge diff to look at, much of it is noise. All the files with headers that have been added should not be part of this merge request. If the header is wrong (which it is), you will have to edit all the files by hand to fix it.

You should create a new branch with only the header adding script. **NOT** the files that have changed.

0) create new branch
1) Create/Edit your script
2) commit your script
3) run your script to test
4) examine output
5) bzr revert # this will
5) submit merge request

#########

There is no reason to have each key word on a serperate line and no reason to have that big if block in your main code. Just define $doc_header with the keywords and pre-pend that.

 google: perl heredoc

##########
The only thing worse than no documention is bad documentation

the header you add should not contain PURPOSE because that HEADER line should make sense and can only be added by hand.

##########

If you run the script more than one, you will create multiple headers.

The logic for this script should be:

if (has_no_header($file)){
    add_header($file);
} else{
   # header is there, nothing need be done
   #

}

########
You are not appending (putting something at the end) you are prepending (putting something at the start)

########
branch names should be in all small letters.

   lp:~himabindu-sanagavarapu/mvhub/ADD_HEADER_TO_MISSING_FILES

no need to fix now, but use small letters in the future (like when you create the new branch above)

############

mv_prove doesn't run, therefore the tests all *****FAIL*****

You ****MUST*** run the tests and they must ***PASS*** before you sumbit a merge request.

##########
The reason they are failing ***AGAIN*** is you've removed the executable bit from all the scripts in app-mvhub/bin/

I'd guess is that you are moving files back and forth from a windows machine, which is fine as long as you don't change things that shouldn't be changed.

cdw is also broken because mv_get_active is no longer executable

##########
The directory

  app-mvhub/bin

...is for files that are part of the application, that the end user will use

The directory:

   app-mvhub/project-tools/bin

...is for tools we use to develop the application.

Our development tools are prefixed "mv_"

Therefore

   app-mvhub/bin/header_add_v1.pl

should be in:

   app-mvhub/projet-tools/bin/mv_add_doc_header

Reply
review: Approve
Dan MacNeil on 2010-09-04
review: Needs Fixing
hsanagavarapu wrote on 2010-09-20

Sir,

Thank you for your detailed review.I have the following doubts in fixing my code.And before doing some changes I just want to make sure also.

> This is a huge diff to look at, much of it is noise. All the files with
> headers that have been added should not be part of this merge request. If the
> header is wrong (which it is), you will have to edit all the files by hand to
> fix it.
>
> You should create a new branch with only the header adding script. **NOT**
> the files that have changed.

> 0) create new branch
> 1) Create/Edit your script
...

[Dan MacNeil (omacneil)]

#######
comments from lp:~himabindu-sanagavarapu/mvhub/header_doc_t ⇒ lp:~mvhub-commit/mvhub/trunk
#######

It looks like many of the corrections requested in the last review have not been made.

For example here:

24 +sub _search_string_in_file {

The speed of the test, the structure of the main block,etc, etc

Generally, when you reject corrections, you should comment as to why.

[Dan MacNeil (omacneil)]

I'm looking at lp:~himabindu-sanagavarapu/mvhub/header_add_files_519126

Some problems:

  1) You didn't merge trunk back into your branch before pushing.
  2) your script mv_header_add.pl turns every file in the project to garbage.

Instructions for merging trunk back into your branch are in the cheatsheet. One big problem with not merging trunk back is that I have to look at a much bigger diff than I need to. All the changes made to trunk since your branch was made are showing up.

           bzr diff --old /var/www/mvhub/omacneil/source-code/trunk/ | more

In regard to the garbage:
 1) Look at a file CONTRIBUTORS for example
 2) run your script
 3) look at CONTRIBUTORS again

As a rule for life, you should take a look at the output of your scripts after they run.

When you work in our environment, you should run the tests before asking for review. Either of these rules would have let you catch the problem

[hsanagavarapu (himabindu-sanagavarapu)]

Hi,

Thank you for spending time in reviewing my code.

My old branch header_add_files_519126 is creating garbage,that I have realised it a little later and created new branch which is add_header_files_519126. I have informed this by email but missed to update in the bug. Sorry about that.

I just realized that my code is failing for the shell scripts with shebang such as #!/bin/bash.
I'll push the branch after fixing it and let you know.

Thank you
Himabindu

[hsanagavarapu (himabindu-sanagavarapu)]

Hi,

I removed my unused branch which is Missing_Header_Bug_519126.Can you please have a look at my branch code(header_doc_t)

Thank you
Himabindu

[Dan MacNeil (omacneil)]

you need to have all the tests pass before running mv_header_add.pl

---This is not the case perltidy fails

You need to have all the tests pass after running mv_header_add.pl

----many of them fail after running your script

You need to look at your test site http://mvh.hsanagav.testing123.net after you run your script.

you need to decide the comment type based on $file_type not extension

You need to:

 1) spit out an error with filename
 2) ***DO NOTHING***

...for file you don't know how to handle.

Think of using sub routines.

Use a data structure not an if block

[Dan MacNeil (omacneil)]

#!/usr/bin/perl

use strict;
use warnings;

{ # main

    # see man perlvar for @ARGV
    my $filename = $ARGV[0]
      or die "need to supply a filename argument\n";
    my $type = get_type_from($filename);
    my %file = make_type_table();

    if ( !$file{$type} ) {
        print "can't handle file type: \n\t'$type' \nin file: \n\t'$filename'\n";
    }
    else {
       print "ok\n";
    }

}

sub get_type_from {
    my $filename = shift;
    my $type = `file -b $filename`;
    chomp $type;
    $type =~ s/^\s+//;

    return $type;
}

sub make_type_table {
    my %f = (
        'ASCII English text' => {
            start => '#',
            stop => '',
        },
        'ASCII HTML document text' => {
            start => '<!-- ',
            stop => '-->',
        },
    );

    return %f;
}

# make_success_output($filename,$type,
# $file{$type}{start},
# $file{$type}{stop});

sub make_success_output {
my ($filename,$type,$start,$end)=@_;
my $result = << "HERE_DOC";
File: '$filename'";
print "Type: '$type'";
   comment line starts with: is '$start';
   comment line ends with: is '$end';
HERE_DOC

}

[troylamonte (troylamonte)]

Blessing and honor to our Lord and Savior Jesus Christ

                                                                    Troy L
Thompson
*'A veteran - one of many who, at one point in his or her life, wrote a
blank check made payable to The 'United States of America', for an amount
of 'up to and including their life.' (Author unknown)*

---------- Forwarded message ----------
From: Colonel John W. Tilford <email address hidden>
Date: Tue, Nov 12, 2013 at 7:33 PM
Subject: Thank you for signing the ""Space Available" eligibility for 100%
service connected disabled veterans" petition
To: troy thompson <email address hidden>

Thank you for signing my petition, *"Space Available" eligibility for 100%
service connected disabled veterans.*

As of now, the petition has received 252 signatures! To really make a
difference, we need a lot more people to join in. Can you share this
petition with all your friends?

Click here to share it on Facebook:

Share on Facebook<http://petitions.moveon.org/fb_share?user_id=9470878&petition_id=20482&source=s.fb.ty&medium=email&mailing=thank_you>

Then, forward the email below to everyone you know.

Thanks!

—Colonel John W. Tilford

Here's a sample message to send to your friends:

----------------------------------------------------------------------------------

Hi,

Military retirees and their dependents (such as myself and wife) are
eligible for "Space Available" flights on Air Mobility Command aircraft.
Only otherwise wasted seats are used. Of the six category levels of "Space
A" eligibility, I'm only asking that my level, the 6th and lowest, be
expanded to include veterans rated by the Department of Veterans Affairs as
100% service connected disabled. My wife and I have flown to England,
Germany, California, Ohio, Maryland, etc. via Space A as Category 6
passengers. How unfair it is that those who gave everything but their lives
for their Country, that were medically separated and did not have a chance
to retire from the military, are not eligible for Space A. Extending this
benefit to these totally disabled veterans would cost NOTHING! Remember,
only otherwise wasted seats are used for Space A, and all the
administrative systems are already in place and operational.

That's why I signed a petition to The United States House of
Representatives and The United States Senate, which says:

"Expand Air Mobility Command Space Available flight eligibility to include
veterans rated by the Department of Veterans Affairs as 100% service
connected disabled. "

Will you sign the petition too? Click here to add your name:

http://petitions.moveon.org/sign/space-available-eligibility?source=s.fwd&r_by=9470878

Thanks!

------------------------------
 You're receiving this message because you signed the *"Space Available"
eligibility for 100% service connected disabled veterans* petition on the
MoveOn.org <http://moveon.org> petition website. MoveOn Civic Action does
not endorse the contents of this email or the petitions posted on MoveOn's
public petition website. If you don't want to receive e-mail about this
petition, click here to
unsubscribe<http://petitions.moveon.org/unsub.html?i=thanks_20505-9470878-7vd...

[troylamonte (troylamonte)]

Blessing and honor to our Lord and Savior Jesus Christ

                                                                    Troy L
Thompson
*'A veteran - one of many who, at one point in his or her life, wrote a
blank check made payable to The 'United States of America', for an amount
of 'up to and including their life.' (Author unknown)*

---------- Forwarded message ----------
From: Anna Galland, MoveOn.org Political Action <<email address hidden>
>
Date: Thu, Jul 24, 2014 at 1:19 PM
Subject: John Boehner: Enough is enough!
To: troy thompson <email address hidden>

  Sign the petition: Tell Speaker Boehner and House Republicans to stop
wasting millions of our tax dollars on their partisan lawsuit against
President Obama and move on to the real issues facing our country.
<http://www.moveon.org/r/?r=300088&id=98959-27090827-pmN4BEx&t=1>
  <http://www.moveon.org/r/?r=300088&id=98959-27090827-pmN4BEx&t=2> "Speaker
Boehner and House Republicans: Stop wasting millions of our tax dollars on
your partisan lawsuit against President Obama. Please move on to the real
issues facing our country."
<http://www.moveon.org/r/?r=300088&id=98959-27090827-pmN4BEx&t=3>
 Sign the Petition!
<http://www.moveon.org/r/?r=300088&id=98959-27090827-pmN4BEx&t=4>

  Dear MoveOn member,

House Speaker John Boehner just took his next step toward a taxpayer-funded
lawsuit against President Obama. His reason? President Obama has used his
executive authority to get things done, while Congress has dithered.

In a recent membership vote,* 88% of MoveOn members said that if John
Boehner launched this wasteful lawsuit, then holding him accountable should
be a top MoveOn priority.*

So we're fighting back. Together, we can elevate the profile of this
ridiculous lawsuit, expose it as partisan politics at its worst, and ensure
that it reinforces the worst of the GOP brand, backfiring on Republicans in
the November elections.

Will you sign our petition to demonstrate your opposition to this latest
Republican outrage? We've set a goal of 150,000 signatures—and we'll be
reporting the numbers to the media in real time.

The petition says: "Speaker Boehner and House Republicans: Stop wasting
millions of our tax dollars on your partisan lawsuit against President
Obama. Please move on to the real issues facing our country."

*Click here to sign, and then pass it on to your friends.
<http://www.moveon.org/r/?r=300088&id=98959-27090827-pmN4BEx&t=5>*

With elections approaching, this could be political suicide for
Republicans. If we shine a huge spotlight on this insanity, it could drive
down GOP popularity like the government shutdown last fall.* New polling
confirms that a large majority of Americans—including many Republicans—do
not want the GOP wasting millions of our tax dollars on this political
stunt.*1

Yet today, the Republican-controlled House Rules Committee moved forward
with legislation to authorize the lawsuit—laying the groundwork for a vote
on the full House floor next week. Once that happens, Speaker Boehner will
actually file suit.

And then he'll lose. *Virtually every legal expert who's weighed in expects
this lawsuit to be rejected on the merits.*2 But John Boe...