Skip to content

Aggiunte opzioni notify e help, settata la creazione della variabile $MANNAGGIAURL solo se l'audioflag è vera#16

Open
Nhoya wants to merge 3 commits intoLegolasTheElf:masterfrom
Nhoya:master
Open

Aggiunte opzioni notify e help, settata la creazione della variabile $MANNAGGIAURL solo se l'audioflag è vera#16
Nhoya wants to merge 3 commits intoLegolasTheElf:masterfrom
Nhoya:master

Conversation

@Nhoya
Copy link
Copy Markdown

@Nhoya Nhoya commented Feb 13, 2016

Aggiunta opzione per mannaggiare via notifica desktop (--notify) e la pagina di aiuto (--help)

P.S. che ne pensate di riscriverlo in bash?

Comment thread mannaggia.sh Outdated
else
notify-send "$MANNAGGIA" 2>/dev/null
fi
elif [ "$help" = 1 ]; then
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.

Da un punto di vista puramente logico, se l'utente chiede l'help ogni azione dovrebbe essere soppressa, quindi il check dovrebbe essere in cima

@rfc1459
Copy link
Copy Markdown
Contributor

rfc1459 commented Feb 13, 2016

I commenti specifici sono inline nel commit, ma in generale per me è un -1:

  • due funzionalità completamente unrelated infilate nella stessa pull request
  • la parte sull'help è farcita di bashismi facilmente evitabili usando printf (shellcheck è vostro amico)

@Nhoya
Copy link
Copy Markdown
Author

Nhoya commented Feb 13, 2016

Ho trovato inutile mandare due pullrequest differenti per 4 righe di codice, per quanto riguada i bashismi, eh.. mea culpa

@Nhoya
Copy link
Copy Markdown
Author

Nhoya commented Feb 13, 2016

Sistemati tutti i problemi sopra, shellcheck passato, penso manchi solo la stretta di mano del signor Bourne

@rfc1459
Copy link
Copy Markdown
Contributor

rfc1459 commented Feb 14, 2016

LGTM, per la stretta di mano da parte di Bourne ci stiamo attrezzando :)

@Nhoya Nhoya changed the title Aggiunte opzioni notify e help Aggiunte opzioni notify e help, settata la creazione della variabile $MANNAGGIAURL solo se l'audioflag è vera Feb 14, 2016
@Nhoya
Copy link
Copy Markdown
Author

Nhoya commented Feb 14, 2016

Ho già pronto il nuovo branch con l'opzione --day per i santi del giorno, appena mi date un resoconto su questa pull request mando l'altra

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants