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

Deprecate List module, introduce LinkedLists #12454

Merged
merged 10 commits into from
Mar 5, 2019

Conversation

benharsh
Copy link
Member

@benharsh benharsh commented Mar 3, 2019

This PR adds a deprecation warning to the "List" module and introduces a new "LinkedLists" module to take its place, and likewise renames the type from "list" to "LinkedList". We expect to eventually use the name "list" (along with "deque") for a type to replace some of the array-as-vector operations.

Modules and tests have been updated to use the new LinkedList type, and a deprecation test has been added.

Testing:

  • local
  • gasnet

Copy link
Member

@ben-albrecht ben-albrecht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

modules/standard/List.chpl Outdated Show resolved Hide resolved
modules/standard/List.chpl Outdated Show resolved Hide resolved
@vasslitvinov
Copy link
Member

My preference is to make the name capitalized: LinkedList.

I think this is our convention for library types that are not all-lowercase like bigint or date.

To me linkedList looks more like the name of a variable.

To avoid the potential conflict with the module name, we could follow the lead of the Barrier type, whose module is plural Barriers. In this case, the module name would be LinkedLists. Allowing us to add a doulbly-linked list to it easily later.

@ben-albrecht
Copy link
Member

@vasslitvinov makes a good point.

The closest thing to documented guidelines on this is in #6698 :

Types should be CamelCase or lowercase. User-defined types should generally start with an uppercase letter and be CamelCase (e.g. RandomStream,Timer). Internal types can be lowercase (e.g. int, domain,bigint)

I don't think a linked list counts as an internal type, so I agree that LinkedList type and LinkedLists.chpl is more consistent.

@benharsh
Copy link
Member Author

benharsh commented Mar 4, 2019

I've pushed some commits that use LinkedList/LinkedLists and will re-test.

@bradcray : do you have a preference?

Edit: If we end up preferring "linkedList" for now, I can just revert the commits.

@bradcray
Copy link
Member

bradcray commented Mar 5, 2019

No particular preference on naming here.

@benharsh benharsh changed the title Deprecate List module, introduce LinkedList Deprecate List module, introduce LinkedLists Mar 5, 2019
@benharsh benharsh merged commit 6a092ff into chapel-lang:master Mar 5, 2019
@benharsh benharsh deleted the deprecate-linked-list branch August 7, 2022 17:03
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.

4 participants