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

GSoC 2023: Aryan Gupta week 4 #310

Merged
merged 3 commits into from
Jun 25, 2023
Merged

Conversation

aryan1010
Copy link
Contributor

Fixes # .

Changes proposed in this pull request:
-added a new structure path_rtdd
-made some new v4 files
-adjusted function names

@pgRouting/admins

@aryan1010 aryan1010 merged commit 658ce75 into pgRouting:aryan-2023 Jun 25, 2023
4 of 10 checks passed
Copy link
Member

@krashish8 krashish8 left a comment

Choose a reason for hiding this comment

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

Left some comments.

It is okay for now to have different v4 files for your functionality, but in the end, if time allows, we would not want to have separate files. Separate files for some functionality lead to duplicated code.

.vscode
get_signatures.sh
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be removed when the final PR is created for merging to the pgRouting repository.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

********************************************************************PGR-GNU*/
/*! @file */

#ifndef INCLUDE_C_TYPES_path_rtdd_H_
Copy link
Member

Choose a reason for hiding this comment

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

Header Include guards should be all-caps. I guess that clang would report a warning in this case.

# include <stdint.h>
#endif

struct path_rtdd {
Copy link
Member

Choose a reason for hiding this comment

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

A better struct naming (and file naming) would be one that has rt (return type) at the end. E.g. path_dd_rt or pathdd_rt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay got that

@@ -0,0 +1,489 @@
/*PGR-GNU*****************************************************************

file: driveDist.hpp
Copy link
Member

Choose a reason for hiding this comment

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

Fix the file name

Comment on lines +8 to +17
Copyright (c) 2022 Celia Virginia Vergara Castillo
Copyright (c) 2015 Celia Virginia Vergara Castillo
vicky at erosion.dev

Copyright (c) 2023 Aryan Gupta
guptaaryan1010 AT gmail.com

Copyright (c) 2020 The combinations_sql signature is added by Mahmoud SAKR
and Esteban ZIMANYI
mail: m_attia_sakri at yahoo.com, estebanzimanyi at gmail.com
Copy link
Member

Choose a reason for hiding this comment

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

In case you are creating a new file, you need not include the other contributors - just project at pgrouting.org and your email is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay sir

********************************************************************PGR-GNU*/

#ifndef INCLUDE_DRIVERS_DRIVING_DISTANCE_v4drivedist_driver_H_
#define INCLUDE_DRIVERS_DRIVING_DISTANCE_v4drivedist_driver_H_
Copy link
Member

Choose a reason for hiding this comment

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

Header include guard should be all-caps

@@ -27,7 +27,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

-- MULTIPLE
--v3.6
CREATE FUNCTION pgr_drivingDistance(
CREATE FUNCTION pgr_v4drivingDistance(
Copy link
Member

Choose a reason for hiding this comment

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

Any user facing function (aka, the non-underscored one) should not have v4 prefix. All end-user facing function names should be consistent - they should be named similar to others.

The underscore functions can have any name - e.g. the v4 prefix, because we don't expect the end users to use these functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OKAY got that

@@ -53,7 +53,7 @@ ROWS 1000;

-- SINGLE
--v3.6
CREATE FUNCTION pgr_drivingDistance(
CREATE FUNCTION pgr_v4drivingDistance(
Copy link
Member

Choose a reason for hiding this comment

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

Same as above comment

@@ -0,0 +1,229 @@
/*PGR-GNU*****************************************************************
File: drivedist_driver.cpp
Copy link
Member

Choose a reason for hiding this comment

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

Fix file name

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

Successfully merging this pull request may close these issues.

None yet

2 participants