Skip to content
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

Contributing guidelines #28

Open
benthestatistician opened this issue Feb 18, 2022 · 11 comments
Open

Contributing guidelines #28

benthestatistician opened this issue Feb 18, 2022 · 11 comments
Labels
documentation Improvements or additions to documentation

Comments

@benthestatistician
Copy link
Contributor

benthestatistician commented Feb 18, 2022

An issue for discussion about guidelines for contributing to the package.

Proposed guidelines can be added to the Contributing section of README.md in the corresponding branch, i28-contributing, which should be merged back into the main branch if/as consensus emerges. (If consensus doesn't emerge then there can be offshoot branches, e.g. i28-contributing-a, i28-contributing-b, ...)

benthestatistician added a commit that referenced this issue Feb 18, 2022
Actually 2 sugestions:
1. Avoid committing whitespace at ends of lines
2. Avoid committing whitespace *changes*

Cf. #28
@benthestatistician
Copy link
Contributor Author

@josherrickson --

  1. what do you think of my proposal in a05f007? If you support, pls merge into main. If you have comments or concerns, pls note here.
  2. That suggestion doesn't speak directly to trailing lines (as opposed to trailing spaces at end of line). I held off on that b/c I saw that when I save a file without a trailing line, git diff seems to throw a warning flag. E.g. my diff for 14dc077:
@@ -34,3 +34,4 @@ Optionally, we can also include a covariance adjustment model through the
     covadjmod <- lm(y ~ x1 + x2 + ..., data = studentdata, subset = !txt)
     lm(y ~ txt, studentdata, weights = ett(des),
        offset = cov_adj(covadjmod, data = studentdata)
+      )
\ No newline at end of file

I take it your pref is not to commit trailing lines. If you see the trailing newline as problematic for git blame (or something else), could you explain a bit, or point us to discussion somewhere on the web?

@josherrickson
Copy link
Collaborator

I like those changes, I'll merge them in presently.

I don't think that's a warning/flag in the diff, its just noting the lack of a newline. I don't see that as causing any issues?

@josherrickson
Copy link
Collaborator

I copied over the text from optmatch's contribution as a start.

@josherrickson josherrickson added the documentation Improvements or additions to documentation label Feb 21, 2022
@josherrickson
Copy link
Collaborator

lintr and styler may come in handy if we're willing to commit the time to set up a custom style guide that works for us.

@benthestatistician
Copy link
Contributor Author

I have some (minor) issues with line endings, having to do w/ my occasional use of a Windows computer. Anybody have concerns about my committing a .gitattributes file that reads

* text=auto

per this rec?

@josherrickson
Copy link
Collaborator

Wouldn't the easier solution be to stop using Windows?

Sarcasm aside, sounds like a good idea to me.

@jwasserman2
Copy link
Collaborator

The paragraph How to write good commit messages here specifies a good convention for labeling commits by their purpose. Since we prefer small commits, the larger text body suggestion would probably just be a pain (as someone who commits from the command line), so the label + a succinct description of what the commit achieves is enough for the commit message I think

benthestatistician added a commit that referenced this issue Jun 2, 2022
@benthestatistician
Copy link
Contributor Author

All, As context for @jwasserman2's comment just above, I had asked him if he'd put a link here to documentation behind the commit conventions he follows.

JW, could you give us a listing of the prefixes you use to flag different types of commits, something analogous to the listing under "1. Specify the type of commit" in the document you linked us to?

If so, we might use that as a starting point for discussion of such conventions that we might like to observe for this project.

@jwasserman2
Copy link
Collaborator

The paragraph How to write good commit messages here specifies a good convention for labeling commits by their purpose. Since we prefer small commits, the larger text body suggestion would probably just be a pain (as someone who commits from the command line), so the label + a succinct description of what the commit achieves is enough for the commit message I think

Particularly, I use the tags for the following purposes:

  • BUG: commit fixes a bug
  • ADD: commit adds a new feature
  • ENH: commit enhances some existing functionality
  • STY: commit changes syntax used or maybe the internal order of operations for functions but does not affect their outputs or how a user interfaces with them
  • DOC: commit updates vignettes, the README, or function documentation
  • TST: commit adds new tests for existing functionality
  • MERGE: commit fixes git conflicts to allow for a merge

@josherrickson
Copy link
Collaborator

josherrickson commented Jun 29, 2022

Added a discussion of using () when linking to functions in the docs.

@benthestatistician
Copy link
Contributor Author

I'd like to add another tag to @jwasserman2's list above, for commits that only update a specification document, or fill out inline comments or something like that. At first pass (e.g. 89179e6) I'm going to go with "DEVNOTE", but I'm open to other suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants