Skip to content
This repository was archived by the owner on Oct 23, 2025. It is now read-only.

clarify CATKIN_IGNORE may be a file, directory, or (dangling) symlink#256

Merged
dirk-thomas merged 3 commits intoros-infrastructure:masterfrom
rotu:patch-5
May 4, 2020
Merged

clarify CATKIN_IGNORE may be a file, directory, or (dangling) symlink#256
dirk-thomas merged 3 commits intoros-infrastructure:masterfrom
rotu:patch-5

Conversation

@rotu
Copy link
Copy Markdown
Contributor

@rotu rotu commented Apr 29, 2020

Mention that CATKIN_IGNORE may be any type of file and its contents are ignored

Mention that CATKIN_IGNORE may be any type of file
change "are ignored" to "don't matter" to avoid implying that that's what "IGNORE" is referring to in "CATKIN_IGNORE"
Copy link
Copy Markdown
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

Beside the inline question please update the title of the PR since it will become the commit message.

@rotu rotu changed the title Update rep-0128.rst CATKIN_IGNORE may be any type of file Apr 29, 2020
rotu added a commit to rotu/colcon-core that referenced this pull request Apr 29, 2020
Copy link
Copy Markdown
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

Widening the possible types for the marker sounds reasonable to me. I don't expect any breakage being introduced due to this change (like users have directories or (dangling) symlinks with this name but not intending to ignore the path).

@dirk-thomas
Copy link
Copy Markdown
Member

@ros-infrastructure/ros_team Please review and approve this proposed change.

Copy link
Copy Markdown

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

I don't see anything wrong with this, but I am curious how you use it in practice? Is there a benefit to making the marker files a directory or symlink?

rotu added a commit to rotu/colcon-core that referenced this pull request Apr 29, 2020
@mjcarroll
Copy link
Copy Markdown

Is there a benefit to making the marker files a directory or symlink?

Answered my own question: ros-infrastructure/catkin_pkg#286 (comment)

Copy link
Copy Markdown
Contributor

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM

@dirk-thomas dirk-thomas changed the title CATKIN_IGNORE may be any type of file clarify CATKIN_IGNORE may be a file, direcotry, or (dangling) symlink May 4, 2020
@dirk-thomas dirk-thomas changed the title clarify CATKIN_IGNORE may be a file, direcotry, or (dangling) symlink clarify CATKIN_IGNORE may be a file, directory, or (dangling) symlink May 4, 2020
@dirk-thomas
Copy link
Copy Markdown
Member

3 👍 and 5 days sounds like enough to me. Merging...

@dirk-thomas dirk-thomas merged commit c1e8136 into ros-infrastructure:master May 4, 2020
dirk-thomas pushed a commit to colcon/colcon-core that referenced this pull request May 4, 2020
* Allow ignore_marker to be a dangling symlink

As per ros-infrastructure/rep#256

* os.path.lexists instead of (nonexistent) pathlib.path.lexists
prajakta-gokhale pushed a commit to colcon/colcon-bundle that referenced this pull request May 13, 2020
* Respect ignore marker even if dangling symlink

As per ros-infrastructure/rep#256

* os.path.lexists instead of (nonexistent) pathlib.path.lexists

Co-authored-by: Prajakta Gokhale <prajaktg@amazon.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants