feat - Add new file name guidelines #638

bhaumikmistry · 2019-11-25T15:55:07Z

Description of Change

Added new file name guidelines, the new contribution needs to be in certain format to be used for the dynamic directory list generator. The file names will be validated using the actions in near future.

Checklist

Added description of change
Relevant documentation/comments is changed or added
PR title follows semantic commit guidelines
Search previous suggestions before making a new one, as yours may be a duplicate.
Sort by alphabetical order
I acknowledge that all my contributions will be made under the project's license.

Notes:
The old file names will be changes soon with one directory at a time.
The old PR might need some re-work.

cclauss · 2019-11-27T13:57:25Z

Do we want uppercase directory names?
Do we want directory names that contain " " or "-"?
Do we need to test if a pull request creates a new directory? We don't want arbitrary directory names.

cclauss · 2019-11-27T14:01:43Z

CONTRIBUTION.md

+- Use lowercase words with ``"_"`` as separator
+- For instance 
+```
+MyNewCppClass.cpp       is incorrect


Let's make this MyNewCppClass.CPP because we also care about the file extension (.cpp).

Yes, agreed

cclauss · 2019-11-27T14:03:26Z

CONTRIBUTION.md

+```
+- It will be used to dynamically create a directory of files and implementation.
+- File name validation will run on docker to ensure the validity.
+
 #### Commit Guidelines
 - It is recommended to keep your changes grouped logically within individual commits. Contributors find it easier to review changes that are silt across multiple commits.


silt --> split

bhaumikmistry · 2019-11-27T15:21:22Z

Do we want uppercase directory names?
Do we want directory names that contain " " or "-"?
Do we need to test if a pull request creates a new directory? We don't want arbitrary directory names.
I have already worked on a cpp program which we can run in action which would validate the directory names for the rules that I have added in the new patch along with the file names guidelines. However for the new changes to be detected, we need to fix the current names of directory and files do you recommend anything for that? To make it easy and quick

@cclauss

you have already added an action to check the file name, lets have no space and - for the directory names to make it best practice and dynamically create a list as well

cclauss · 2019-11-27T16:17:19Z

So we will need a mega-rename pull request that renames all directories and all filenames?

bhaumikmistry · 2019-11-27T17:55:30Z

So we will need a mega-rename pull request that renames all directories and all filenames?

I guess so. Finally, any name(directory or file) cannot contain space and '-' , also only lower chars.
I think a pull request per directory would make more sense and would be stable and/or more structural.

cclauss · 2019-11-28T09:32:52Z

All new or modified C++ files in a pull request will be tested by cpplint in https://github.com/TheAlgorithms/C-Plus-Plus/blob/master/.github/workflows/cpplint_modified_files.yml and if they pass cpplint, they will also be tested for " ", "-", uppercase characters, and that they must be in directories.

cclauss · 2019-11-28T10:15:34Z

One of the cpplint tests looks for a copyright comment in the file like
https://github.com/TheAlgorithms/C-Plus-Plus/blob/master/Others/happy_number.cpp#L4

Do we want to encourage the addition of such comments or do we want to disable that test?

Other tests to consider: cpplint --filter=

  build/class
  build/c++11
  build/c++14
  build/c++tr1
  build/deprecated
  build/endif_comment
  build/explicit_make_pair
  build/forward_decl
  build/header_guard
  build/include
  build/include_subdir
  build/include_alpha
  build/include_order
  build/include_what_you_use
  build/namespaces_literals
  build/namespaces
  build/printf_format
  build/storage_class
  legal/copyright
  readability/alt_tokens
  readability/braces
  readability/casting
  readability/check
  readability/constructors
  readability/fn_size
  readability/inheritance
  readability/multiline_comment
  readability/multiline_string
  readability/namespace
  readability/nolint
  readability/nul
  readability/strings
  readability/todo
  readability/utf8
  runtime/arrays
  runtime/casting
  runtime/explicit
  runtime/int
  runtime/init
  runtime/invalid_increment
  runtime/member_string_references
  runtime/memset
  runtime/indentation_namespace
  runtime/operator
  runtime/printf
  runtime/printf_format
  runtime/references
  runtime/string
  runtime/threadsafe_fn
  runtime/vlog
  whitespace/blank_line
  whitespace/braces
  whitespace/comma
  whitespace/comments
  whitespace/empty_conditional_body
  whitespace/empty_if_body
  whitespace/empty_loop_body
  whitespace/end_of_line
  whitespace/ending_newline
  whitespace/forcolon
  whitespace/indent
  whitespace/line_length
  whitespace/newline
  whitespace/operators
  whitespace/parens
  whitespace/semicolon
  whitespace/tab
  whitespace/todo

bhaumikmistry · 2019-12-02T15:11:44Z

One of the cpplint tests looks for a copyright comment in the file like
https://github.com/TheAlgorithms/C-Plus-Plus/blob/master/Others/happy_number.cpp#L4

Do we want to encourage the addition of such comments or do we want to disable that test?

I think we are okay with not adding at this point. @cclauss

cclauss · 2019-12-02T15:44:51Z

Removed legal/copyright https://github.com/TheAlgorithms/C-Plus-Plus/blob/master/.cpplint#L4

cclauss · 2019-12-02T15:47:12Z

CONTRIBUTION.md

+- File name validation will run on docker to ensure the validity.
+
+#### New Directory guidelines 
+- Use lowercase words with ``"_"`` as separator ( no spaces or '-' allowed )


Do we want to say that it is best practice to try to use an existing directory rather than adding a new one?

added change

cclauss · 2019-12-02T15:49:53Z

.github/pull_request_template.md

@@ -10,7 +10,8 @@ Contributors guide: https://github.com/TheAlgorithms/C-Plus-Plus/CONTRIBUTION.md
 <!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

 - [ ] Added description of change
- [ ] Added tests and example, test passes
+- [ ] Added file name matches [File name guidelines](https://github.com/TheAlgorithms/C-Plus-Plus/blob/master/CONTRIBUTION.md#New-File-Name-guidelines)
+- [ ] Added tests and example, test must pass


Do you have some existing tests in this repo that we could site as examples?

Do we want to have a check box for:

Ran cpplint, test must pass

cpplint is run after the PR is created so the dev would have to edit the PR description after the checks have passed. Also working on a example test case and helper test cpp methods to be used for later and different pr.

cpplint can (and should) be run on their local machine.

then need add guideline or script to do so?

add new directory updates.

cclauss · 2019-12-02T18:05:36Z

CONTRIBUTION.md

@@ -38,7 +38,8 @@ my_new_cpp_class.cpp    is correct format
 - File name validation will run on docker to ensure the validity.

 #### New Directory guidelines 
- Use lowercase words with ``"_"`` as separator ( no spaces or '-' allowed )
+- We recommend adding files to existing directories as mush as possible.


mush --> much

spell change for # new directory guidelines

bhaumikmistry · 2019-12-03T15:48:10Z

@cclauss Approval?

feat/ add new file name guidelines

f8e1cb3

bhaumikmistry assigned ashwek and cclauss Nov 25, 2019

This was referenced Nov 26, 2019

GitHub Action to run cpplint on all pull requests #639

Merged

Add GitHub Action to validate filenames #640

Closed

cclauss reviewed Nov 27, 2019

View reviewed changes

bhaumikmistry added 2 commits November 27, 2019 10:26

add new guidelines

7a8b5c9

add directory guidelines

194b039

cclauss reviewed Dec 2, 2019

View reviewed changes

Update CONTRIBUTION.md

cacd1eb

add new directory updates.

cclauss reviewed Dec 2, 2019

View reviewed changes

Update CONTRIBUTION.md

d8f55f2

spell change for # new directory guidelines

cclauss approved these changes Dec 3, 2019

View reviewed changes

bhaumikmistry merged commit f2b8757 into TheAlgorithms:master Dec 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat - Add new file name guidelines #638

feat - Add new file name guidelines #638

bhaumikmistry commented Nov 25, 2019

cclauss commented Nov 27, 2019

cclauss Nov 27, 2019

bhaumikmistry Nov 27, 2019

cclauss Nov 27, 2019

bhaumikmistry commented Nov 27, 2019 •

edited

Loading

cclauss commented Nov 27, 2019

bhaumikmistry commented Nov 27, 2019 •

edited by cclauss

Loading

cclauss commented Nov 28, 2019

cclauss commented Nov 28, 2019 •

edited

Loading

bhaumikmistry commented Dec 2, 2019

cclauss commented Dec 2, 2019

cclauss Dec 2, 2019

bhaumikmistry Dec 2, 2019 •

edited

Loading

cclauss Dec 2, 2019

bhaumikmistry Dec 2, 2019

cclauss Dec 2, 2019

bhaumikmistry Dec 2, 2019

cclauss Dec 2, 2019

bhaumikmistry commented Dec 3, 2019

feat - Add new file name guidelines #638

feat - Add new file name guidelines #638

Conversation

bhaumikmistry commented Nov 25, 2019

Description of Change

Checklist

cclauss commented Nov 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhaumikmistry commented Nov 27, 2019 • edited Loading

cclauss commented Nov 27, 2019

bhaumikmistry commented Nov 27, 2019 • edited by cclauss Loading

cclauss commented Nov 28, 2019

cclauss commented Nov 28, 2019 • edited Loading

bhaumikmistry commented Dec 2, 2019

cclauss commented Dec 2, 2019

Choose a reason for hiding this comment

bhaumikmistry Dec 2, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhaumikmistry commented Dec 3, 2019

bhaumikmistry commented Nov 27, 2019 •

edited

Loading

bhaumikmistry commented Nov 27, 2019 •

edited by cclauss

Loading

cclauss commented Nov 28, 2019 •

edited

Loading

bhaumikmistry Dec 2, 2019 •

edited

Loading