Clean byte-compile and checkdoc warnings #8

Merged
sergiruiztrepat merged 1 commits from master into master 2021-06-02 14:35:37 +00:00
sergiruiztrepat commented 2021-06-02 14:24:03 +00:00 (Migrated from github.com)

Hi, I've known about this package in reddit and I like it very much. Congratulations!

I would like to help you to put it on melpa.

In my first review, I have not seen any bug. My byte-compile (Emacs 28.0) throw this warnings:

In screenshot:
screenshot.el:75:11: Warning: ‘screenshot-transient’ is for
interactive use only.

In screenshot-save:
screenshot.el:256:1: Warning: Unused lexical argument `args'

In screenshot-save-as:
screenshot.el:267:1: Warning: Unused lexical argument `args'

In screenshot-copy:
screenshot.el:275:1: Warning: Unused lexical argument `args'

In screenshot-upload:
screenshot.el:293:1: Warning: Unused lexical argument `args'

In end of data:
screenshot.el:375:7: Warning: the function ‘ivy-read’ is not known to
be defined.
screenshot.el:374:17: Warning: the function ‘counsel-fonts’ is not
known to be defined.

After little changes I have no warnings in byte-compile, checkdoc and package-lint.

I would like to know if there are more things to do in this packages so I help to improve it or to put it on Melpa (although it's fine!).

Sergi

Hi, I've known about this package in [reddit](https://www.reddit.com/r/emacs/comments/ngbt2j/offer_collaboration/) and I like it very much. Congratulations! I would like to help you to put it on melpa. In my first review, I have not seen any bug. My byte-compile (Emacs 28.0) throw this warnings: In screenshot: screenshot.el:75:11: Warning: ‘screenshot-transient’ is for interactive use only. In screenshot-save: screenshot.el:256:1: Warning: Unused lexical argument `args' In screenshot-save-as: screenshot.el:267:1: Warning: Unused lexical argument `args' In screenshot-copy: screenshot.el:275:1: Warning: Unused lexical argument `args' In screenshot-upload: screenshot.el:293:1: Warning: Unused lexical argument `args' In end of data: screenshot.el:375:7: Warning: the function ‘ivy-read’ is not known to be defined. screenshot.el:374:17: Warning: the function ‘counsel-fonts’ is not known to be defined. After little changes I have no warnings in byte-compile, checkdoc and package-lint. I would like to know if there are more things to do in this packages so I help to improve it or to put it on Melpa (although it's fine!). Sergi
tecosaur commented 2021-06-02 14:35:28 +00:00 (Migrated from github.com)

Thank you very much @sergiruiztrepat! Your help is much appreciated.

With regards to other areas that could benefit from attention, other than making it compile without warnings for MELPA the main thing I haven't been able to work out is how to deal with wrapped lines. At the moment lines are cut off (i.e. if one line is wrapped to three lines, only the first of the wrapped lines will be captured by the screenshot).

Thanks again!

Thank you very much @sergiruiztrepat! Your help is much appreciated. With regards to other areas that could benefit from attention, other than making it compile without warnings for MELPA the main thing I haven't been able to work out is how to deal with wrapped lines. At the moment lines are cut off (i.e. if one line is wrapped to three lines, only the first of the wrapped lines will be captured by the screenshot). Thanks again!
tecosaur commented 2021-06-02 15:32:55 +00:00 (Migrated from github.com)

Oh, unfortunately it seems like this hasn't cleared away the byte-compiler warnings: see https://github.com/tecosaur/screenshot/runs/2728846335?check_suite_focus=true

Oh, unfortunately it seems like this hasn't cleared away the byte-compiler warnings: see https://github.com/tecosaur/screenshot/runs/2728846335?check_suite_focus=true
sergiruiztrepat commented 2021-06-03 15:28:19 +00:00 (Migrated from github.com)

Hi @tecosaur ,

Ok, I think first work is to clean byte-compile warnings, so I will do it.

Sorry, but I don't know what do you mean with "wrapped lines".

Hi @tecosaur , Ok, I think first work is to clean byte-compile warnings, so I will do it. Sorry, but I don't know what do you mean with "wrapped lines".
Sign in to join this conversation.
No description provided.