New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recipe for the Markless major mode package #5897

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@Shinmera
Copy link
Contributor

Shinmera commented Jan 3, 2019

Brief summary of what the package does

This package implements a major made for the Markless document markup standard.

Direct link to the package repository

https://github.com/shirakumo/markless.el

Your association with the package

I am the maintainer.

Relevant communications with the upstream package maintainer

None needed

Checklist

Please confirm with x:

@riscy
Copy link
Contributor

riscy left a comment

I experimented with some Markless files in this mode and found the support to be quite good. Some comments I have on the markless.el code:

  • Is it worth adding a LICENSE and/or README files to your repository? I had to go googling to find out what Artistic 2.0 is.
  • package-lint:
    47:1: error: "--markless-defface" doesn't start with package's prefix "markless".
    
    It's convention to define "private" functions/variables as e.g. "markless--defface"
  • (byte-compile-file "markless.el")
    In markless-parse-option:
    markless.el:136:37:Warning: function ‘subseq’ from cl package called at
        runtime
    markless.el:142:23:Warning: function ‘subseq’ from cl package called at
        runtime
    markless.el:138:34:Warning: function ‘subseq’ from cl package called at
        runtime
    markless.el:139:62:Warning: function ‘subseq’ from cl package called at
        runtime
    markless.el:139:62:Warning: function ‘subseq’ from cl package called at
        runtime
    markless.el:147:28:Warning: reference to free variable ‘markless-url-regex’
    
    In markless-fontify-url:
    markless.el:303:28:Warning: reference to free variable
        ‘markless-mode-mouse-map’
    
    • Define markless-mode-mouse-map at the top of your file (before it's referenced)
    • Define markless-url-regex at the top of your file
    • You should (require 'url-parse) and (require 'thingatpt)
    • cl is deprecated:
      • use cl-subseq instead of subseq
      • use cl-return-from instead of return-from
      • use car instead of first

Shinmera added a commit to Shirakumo/markless.el that referenced this pull request Jan 6, 2019

@Shinmera

This comment has been minimized.

Copy link
Contributor

Shinmera commented Jan 6, 2019

Thanks for the review! The mentioned issues should be addressed now.

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