Skip to content

The Big Cleanup II#1936

Merged
daniel-j-h merged 14 commits intodevelopfrom
the-big-cleanup-2
Feb 13, 2016
Merged

The Big Cleanup II#1936
daniel-j-h merged 14 commits intodevelopfrom
the-big-cleanup-2

Conversation

@daniel-j-h
Copy link
Copy Markdown
Member

@daniel-j-h daniel-j-h commented Jan 28, 2016

wip

MoKob
MoKob reviewed Jan 28, 2016
View reviewed changes
include/util/exception.hpp Outdated
Copy link
Copy Markdown

@MoKob MoKob Jan 28, 2016

Choose a reason for hiding this comment

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

I deem these common header guard names dangerous if they do not at least include some directory/other identifier. Good chance of header guard conflicts.

Copy link
Copy Markdown
Member Author

@daniel-j-h daniel-j-h Jan 28, 2016

Choose a reason for hiding this comment

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

Yes you are right. Will revert to the osrm prefix again. 👍

@TheMarex
Copy link
Copy Markdown
Member

TheMarex commented Jan 29, 2016

@daniel-j-h can we maybe keep the TODO list in this PR? I'm currently trying to split some smaller items out for @karenzshea to learn about C++ compiler errors 😸

@daniel-j-h
Copy link
Copy Markdown
Member Author

daniel-j-h commented Feb 1, 2016

@TheMarex sure! @karenzshea hit me up if you need help in fighting compilers 🔪

@daniel-j-h
Copy link
Copy Markdown
Member Author

daniel-j-h commented Feb 1, 2016

  • RouteParameter should not depend on boost spirit
  • RouteParameter should either be a struct or have getter-setter, not both <- is done this way to act as a Spirit handler, setters are Spirit callbacks
  • Drop the weak coupling of plugins over service name + registration
    -> libosrm interface should be typed like node-osrm
  • Rename external_memory_node.hpp
  • FixedPointNumber is not used anywhere
  • Remove node_id.hpp
  • Remove check-hsgr
  • FIXME xor_fast_has_shorage.hpp only has space for 2^16 nodes
  • coordinate_calculation should be a namespace not struct
  • Remove reference to crc32_processor.hpp from extractor.hpp
  • Make douglas_peuker a function
  • Make PolylineCompressor a function
  • Make PolylineFormatter a function
  • Make RouteNameExtraction a function
  • Remove SearchEngineData, fold into SearchEngine
  • Drop gpx support
  • Make TravelMode an enum class not possible, would increase node sizes b/c enum classes are not bit-packable
  • Rename TurnInstrauctionsClass -> should be a function
  • rename OSRM class -> Engine
  • ComputeAngle should be a free function and in coordinate_calculation
  • unlint util/:
    • floating_point.hpp is it used?
    • interger_range.hpp is it used?
    • create json/ subfolder
  • mercator should be a namespace in coordinate_calculation
  • rename osrm_exception.hpp to exception.hpp
  • check if range_algorithms is used somewhere
  • remove tribool and fold into server code
  • remove xml renderer
  • remove typedefs.hpp -> No global NodeIDs and EdgeIDs !!!
  • replace SimpleLogger with RTD_LOG
  • Move debug_geometry to extractor
  • routing_algorithms -> routing
  • use sort_unique_resite or remove
  • move matching_debug_info to engine/map_matching
  • hash implementation in processing_chain should use util/std_hash
  • replace DistTableWrapper with more generic implementation (2d vector wrapper or something)
  • move TarjanSCC from extractor to util
  • Remove DescriptorTable
  • Rename FixedPointCoordinate -> Coordinate
  • extraction_helper_functions does not follow style guide, should be cpp file
  • lua_function_exists needs to be renamed
  • NodeBasedDynamicGraphFromEdges needs to be renamed
  • read_file_lower_content needs to be renamed
  • abstract routed_options and fold into tools/routed.cpp
  • rename geometry_string test to polyline
  • move duration_parsing to extractor
  • remove the config file option for osrm-extract and osrm-prepare
  • change extractor class name to Extractor
  • HilbertCode should be function
  • polyline_compressor.cpp:33 "left shift of negative value -106", UB: found with -fsanitize=undefined
  • EncodeToBase64 / DecodeFromBase64 refactor, by value, cleanup

@emiltin
Copy link
Copy Markdown
Contributor

emiltin commented Feb 1, 2016

refactor lua profiles, with base class/functions containing code that's reused between profiles?

@daniel-j-h
Copy link
Copy Markdown
Member Author

daniel-j-h commented Feb 1, 2016

@emiltin I would love if you could tackle that. The profiles desperately need some refactoring and small fixes like for this ticket: #1567.

@daniel-j-h daniel-j-h force-pushed the the-big-cleanup-2 branch 2 times, most recently from c51aa8d to 9ee315e Compare February 8, 2016 19:21
@TheMarex
Copy link
Copy Markdown
Member

TheMarex commented Feb 11, 2016

@daniel-j-h can you please rebase + merge this as we shifted to API work?

@daniel-j-h daniel-j-h mentioned this pull request Feb 12, 2016
Closed
27 tasks
@daniel-j-h
Copy link
Copy Markdown
Member Author

daniel-j-h commented Feb 12, 2016

Okay. Ticketing remaining issues up in #1971.

Waiting for Travis, then will merge.

@daniel-j-h
Folds mercator projections into coordinate_calculation
5de8f18
@daniel-j-h
Transforms osrm_exception to exception
64b3680
@daniel-j-h
Properly use typed math constants instead of impl. specific hacks
ef171f3 
PI is not in the stdlib, neither is 1/pi, pi*2 and so on. Instead
of relying on implementations providing these, use properly typed
math constants.

Main benefits:
- portable and
- returns constexpr, for compile-time computation

References:
- http://www.boost.org/doc/libs/1_60_0/libs/math/doc/html/math_toolkit/constants_intro.html
- http://www.boost.org/doc/libs/1_60_0/libs/math/doc/html/math_toolkit/constants.html
@daniel-j-h
Adds a comment as to why the char to typed enum class transformation …
978b70c 
…is not possible
@daniel-j-h
Prefix exception header guard with osrm prefix
60fc486
@daniel-j-h
Removes floating point epsilon comparator
f207d98
@daniel-j-h
Documents why not to use boost::irange in favor of our hand-written i…
fbef77a 
…range
@daniel-j-h
Folds json_* utilities into json subfolder and adapts includes
ec01c2a
@daniel-j-h
Refactors Base64 encoding and decoding, it's almost beautiful now
aac21f9
@daniel-j-h
Ifdef is_trivially_copyable<T> behind gcc >= 5 guard
c9f0158
@daniel-j-h
Revert "Folds json_* utilities into json subfolder and adapts includes"
4b8c0ac 
This reverts commit cd039c6.
@daniel-j-h
Fix headers for JSON abstractions
a6d406d
@daniel-j-h
Copy link
Copy Markdown
Member Author

daniel-j-h commented Feb 12, 2016

Hopefully this should fix Travis. Waiting for ok, then will merge.

@daniel-j-h daniel-j-h merged commit 27fe85a into develop Feb 13, 2016
@TheMarex TheMarex deleted the the-big-cleanup-2 branch February 13, 2016 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@daniel-j-h @TheMarex @emiltin @MoKob